[vs]: Fix learn fdb events after fdb flush event (sonic-net#524)

Before this fix if flush event is received, vlan id in fdb_info was always set to zero, and it should be extracted from bv_id field in fdb_entry, this was causing to not find exact fdb entry in g_fdb_info_set and since those entries were always there, they were not "learned" again

Author: kcudnik

lib/src/sai_redis_fdb.cpp

@@ -19,7 +19,7 @@ sai_status_t internal_redis_flush_fdb_entries(
 
     std::string key = str_object_type + ":" + sai_serialize_object_id(switch_id);
 
-    SWSS_LOG_DEBUG("flush key: %s, fields: %lu", key.c_str(), entry.size());
+    SWSS_LOG_NOTICE("flush key: %s, fields: %lu", key.c_str(), entry.size());
 
     if (g_record)
     {
@@ -76,7 +76,7 @@ sai_status_t internal_redis_flush_fdb_entries(
                 recordLine("F|" + str_status);
             }
 
-            SWSS_LOG_DEBUG("flush status: %d", status);
+            SWSS_LOG_NOTICE("flush status: %d", status);
 
             return status;
         }

saiplayer/saiplayer.cpp

@@ -857,6 +857,76 @@ void performNotifySyncd(const std::string& request, const std::string response)
     // OK
 }
 
+void performFdbFlush(
+        _In_ const std::string& request,
+        _In_ const std::string response)
+{
+    SWSS_LOG_ENTER();
+
+    // 2017-05-13.20:47:24.883499|f|SAI_OBJECT_TYPE_FDB_FLUSH:oid:0x21000000000000|
+    // 2017-05-13.20:47:24.883499|F|SAI_STATUS_SUCCESS
+
+    // timestamp|action|data
+    auto r = swss::tokenize(request, '|');
+    auto R = swss::tokenize(response, '|');
+
+    if (r[1] != "f" || R[1] != "F")
+    {
+        SWSS_LOG_THROW("invalid fdb flush request/response %s/%s", request.c_str(), response.c_str());
+    }
+
+    if (r.size() > 3 && r[3].size() != 0)
+    {
+        SWSS_LOG_NOTICE("%zu %zu, %s", r.size(), r[3].size(), r[3].c_str());
+        // TODO currently we support only flush fdb entries with no attributes
+        SWSS_LOG_THROW("currently fdb flush supports only no attributes, but some given: %s", request.c_str());
+    }
+
+    // objecttype:objectid (object id may contain ':')
+    auto& data = r[2];
+    auto start = data.find_first_of(":");
+    auto str_object_type = data.substr(0, start);
+    auto str_object_id  = data.substr(start + 1);
+
+    sai_object_type_t ot = deserialize_object_type(str_object_type);
+
+    if (ot != SAI_OBJECT_TYPE_FDB_FLUSH)
+    {
+        SWSS_LOG_THROW("expected object type %s, but got: %s on %s",
+                sai_serialize_object_type(SAI_OBJECT_TYPE_FDB_FLUSH).c_str(),
+                str_object_type.c_str(),
+                request.c_str());
+    }
+
+    sai_object_id_t local_switch_id;
+    sai_deserialize_object_id(str_object_id, local_switch_id);
+
+    if (sai_switch_id_query(local_switch_id) != local_switch_id)
+    {
+        SWSS_LOG_THROW("fdb flush object is not switch id: %s, switch_id_query: %s",
+                str_object_id.c_str(),
+                sai_serialize_object_id(sai_switch_id_query(local_switch_id)).c_str());
+    }
+
+    auto switch_id = translate_local_to_redis(local_switch_id);
+
+    // TODO currently we support only flush fdb entries with no attributes
+    sai_status_t status = sai_metadata_sai_fdb_api->flush_fdb_entries(switch_id, 0, NULL);
+
+    // check status
+    sai_status_t expected_status;
+    sai_deserialize_status(R[2], expected_status);
+
+    if (status != expected_status)
+    {
+        SWSS_LOG_THROW("fdb flush got status %s, but expecting: %s",
+                sai_serialize_status(status).c_str(),
+                R[2].c_str());
+    }
+
+    // fdb flush OK
+}
+
 std::vector<std::string> tokenize(
         _In_ std::string input,
         _In_ const std::string &delim)
@@ -1189,6 +1259,25 @@ int replay(int argc, char **argv)
                     performNotifySyncd(line, response);
                 }
                 continue;
+
+            case 'f':
+                {
+                    std::string response;
+
+                    do
+                    {
+                        // this line may be notification, we need to skip
+                        if (!std::getline(infile, response))
+                        {
+                            SWSS_LOG_THROW("failed to read next file from file, previous: %s", line.c_str());
+                        }
+                    }
+                    while (response[response.find_first_of("|") + 1] == 'n');
+
+                    performFdbFlush(line, response);
+                }
+                continue;
+
             case '@':
                 performSleep(line);
                 continue;
@@ -1217,6 +1306,7 @@ int replay(int argc, char **argv)
                 continue; // skip over query responses
             case '#':
             case 'n':
+                SWSS_LOG_INFO("skipping op %c line %s", op, line.c_str());
                 continue; // skip comment and notification
 
             default:

syncd/syncd_notifications.cpp

@@ -324,6 +324,23 @@ bool check_fdb_event_notification_data(
     return result;
 }
 
+bool contains_fdb_flush_event(
+        _In_ uint32_t count,
+        _In_ const sai_fdb_event_notification_data_t *data)
+{
+    SWSS_LOG_ENTER();
+
+    sai_mac_t mac = { 0, 0, 0, 0, 0, 0 };
+
+    for (uint32_t idx = 0; idx < count; idx++)
+    {
+        if (memcmp(mac, data[idx].fdb_entry.mac_address, sizeof(mac)) == 0)
+            return true;
+    }
+
+    return false;
+}
+
 void process_on_fdb_event(
         _In_ uint32_t count,
         _In_ sai_fdb_event_notification_data_t *data)
@@ -468,6 +485,11 @@ void handle_fdb_event(
 
     sai_deserialize_fdb_event_ntf(data, count, &fdbevent);
 
+    if (contains_fdb_flush_event(count, fdbevent))
+    {
+        SWSS_LOG_NOTICE("got fdb flush event: %s", data.c_str());
+    }
+
     process_on_fdb_event(count, fdbevent);
 
     sai_deserialize_free_fdb_event_ntf(count, fdbevent);

vslib/src/sai_vs_fdb.cpp

@@ -135,16 +135,42 @@ sai_status_t internal_vs_flush_fdb_entries(
 
             fdb_info_t fi;
 
+            // will set vlan to 0, we need to recreate that information
             memset(&fi, 0, sizeof(fi));
 
+            // If fdb entry has bv_id set to vlan object type then we can try to get vlan number from
+            // that object and populate vlan_id in fdb_info. If bv_id is bridge object type then vlan
+            // must be zero, since there can be only 1 (assuming) mac address on a given bridge.
+
             sai_deserialize_fdb_entry(it->first, fi.fdb_entry);
 
+            if (sai_object_type_query(fi.fdb_entry.bv_id) == SAI_OBJECT_TYPE_VLAN)
+            {
+                sai_attribute_t attr;
+
+                attr.id = SAI_VLAN_ATTR_VLAN_ID;
+
+                sai_status_t status = vs_generic_get(SAI_OBJECT_TYPE_VLAN, fi.fdb_entry.bv_id, 1, &attr);
+
+                if (status != SAI_STATUS_SUCCESS)
+                {
+                    SWSS_LOG_ERROR("failed to get vlan_id for vlan object: %s",
+                            sai_serialize_object_id(fi.fdb_entry.bv_id).c_str());
+
+                    return SAI_STATUS_FAILURE;
+                }
+
+                SWSS_LOG_INFO("got vlan id %d", attr.value.u16);
+
+                fi.vlan_id = attr.value.u16;
+            }
+
             auto fit = g_fdb_info_set.find(fi);
 
             if (fit == g_fdb_info_set.end())
             {
                 // this may happen if vlan is invalid
-                SWSS_LOG_ERROR("failed to find fdb entry in info set: %s", it->first.c_str());
+                SWSS_LOG_ERROR("failed to find fdb entry in info set: %s, learn for this MAC will be disabled", it->first.c_str());
             }
             else
             {

vslib/src/sai_vs_hostintf.cpp

@@ -225,6 +225,8 @@ void findBridgeVlanForPortVlan(
 
     // iterate via all bridge ports to find match on port id
 
+    bool bv_id_set = false;
+
     for (auto it = objectHash.begin(); it != objectHash.end(); ++it)
     {
         sai_object_id_t bpid;
@@ -289,6 +291,7 @@ void findBridgeVlanForPortVlan(
                     sai_serialize_object_id(bridge_id).c_str(),
                     attr.value.s32);
             bv_id = bridge_id;
+            bv_id_set = true;
         }
         else
         {
@@ -315,13 +318,21 @@ void findBridgeVlanForPortVlan(
                 if (vlan_id == attr.value.u16)
                 {
                     bv_id = vlan_oid;
+                    bv_id_set = true;
                     break;
                 }
             }
         }
 
         break;
     }
+
+    if (!bv_id_set)
+    {
+        SWSS_LOG_WARN("failed to find bv_id for vlan %d and port_id %s",
+                vlan_id,
+                sai_serialize_object_id(port_id).c_str());
+    }
 }
 
 bool getLagFromPort(
@@ -541,6 +552,7 @@ void process_packet_for_fdb_event(
                 return;
             }
 
+            // untagged port vlan (default is 1, but may change setting port attr)
             vlan_id = attr.value.u16;
         }
     }
@@ -586,10 +598,17 @@ void process_packet_for_fdb_event(
 
     if (fi.fdb_entry.bv_id == SAI_NULL_OBJECT_ID)
     {
+        SWSS_LOG_WARN("skipping mac learn for %s, since BV_ID was not found for mac",
+                sai_serialize_fdb_entry(fi.fdb_entry).c_str());
+
         // bridge was not found, skip mac learning
         return;
     }
 
+    SWSS_LOG_INFO("inserting to fdb_info set: %s, vid: %d",
+            sai_serialize_fdb_entry(fi.fdb_entry).c_str(),
+            fi.vlan_id);
+
     g_fdb_info_set.insert(fi);
 
     processFdbInfo(fi, SAI_FDB_EVENT_LEARNED);