[vpp] SAI RIF loopback packet action and pre-existing VPP vslib defect fixes

vslib/vpp/SwitchVpp.cpp

@@ -803,6 +803,38 @@ void SwitchVpp::setPortStats(
     debugSetStats(oid, stats);
 }
 
+void SwitchVpp::setRifStats(
+        _In_ sai_object_id_t oid)
+{
+    SWSS_LOG_ENTER();
+
+    std::string if_name;
+
+    // Resolve the RIF to its backing VPP interface: the port/LAG/sub-port
+    // hwif, or the bridge BVI (bvi<vlan-id>) for a VLAN RIF. Only these are
+    // backed by a VPP hardware interface whose counters we can read here.
+    if (!vpp_get_rif_hwif_name(oid, if_name))
+    {
+        return;
+    }
+
+    vpp_interface_stats_t rif_stats;
+
+    if (vpp_intf_stats_query(if_name.c_str(), &rif_stats) == 0)
+    {
+        std::map<sai_stat_id_t, uint64_t> stats;
+
+        stats[SAI_ROUTER_INTERFACE_STAT_IN_PACKETS] = rif_stats.rx;
+        stats[SAI_ROUTER_INTERFACE_STAT_IN_OCTETS] = rif_stats.rx_bytes;
+        stats[SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS] = rif_stats.tx;
+        stats[SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS] = rif_stats.tx_bytes;
+        stats[SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS] = rif_stats.rx_error;
+        stats[SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS] = rif_stats.tx_error;
+
+        debugSetStats(oid, stats);
+    }
+}
+
 sai_status_t SwitchVpp::getRouteCounterStats(
         _In_ sai_object_id_t oid,
         _Out_ std::map<sai_stat_id_t, uint64_t>& stats,
@@ -1163,6 +1195,10 @@ sai_status_t SwitchVpp::getStatsExt(
     {
         setPortStats(object_id);
     }
+    else if (object_type == SAI_OBJECT_TYPE_ROUTER_INTERFACE)
+    {
+        setRifStats(object_id);
+    }
     else if (object_type == SAI_OBJECT_TYPE_COUNTER)
     {
         std::string route;

vslib/vpp/SwitchVpp.h

@@ -95,6 +95,9 @@ namespace saivs
             void setPortStats(
                     _In_ sai_object_id_t oid);
 
+            void setRifStats(
+                    _In_ sai_object_id_t oid);
+
             sai_status_t getRouteStatsExt(
                     _In_ sai_object_id_t oid,
                     _In_ uint32_t number_of_counters,
@@ -709,6 +712,23 @@ namespace saivs
                     _In_ uint32_t vlan_id,
                     _In_ uint32_t mtu);
 
+            sai_status_t vpp_apply_loopback_action (
+                    _In_ const std::string& ifname,
+                    _In_ int32_t packet_action);
+
+            sai_status_t vpp_set_interface_loopback_action (
+                    _In_ sai_object_id_t object_id,
+                    _In_ uint32_t vlan_id,
+                    _In_ int32_t packet_action);
+
+            sai_status_t vpp_set_rif_loopback_action (
+                    _In_ sai_object_id_t rif_oid,
+                    _In_ int32_t packet_action);
+
+            bool vpp_get_rif_hwif_name (
+                    _In_ sai_object_id_t rif_oid,
+                    _Out_ std::string& ifname);
+
             sai_status_t UpdatePort(
                     _In_ sai_object_id_t object_id,
                     _In_ uint32_t attr_count,

vslib/vpp/SwitchVppFdb.cpp

@@ -862,6 +862,15 @@ sai_status_t SwitchVpp::vpp_create_bvi_interface(
         interface_set_state(tap_name.c_str(), true);
     }
 
+    // Create-path parity with vpp_update_router_interface: if the VLAN RIF is created
+    // with SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION, apply it on the BVI now.
+    // Otherwise the action would only take effect on a later set.
+    auto attr_loopback = sai_metadata_get_attr_by_id(SAI_ROUTER_INTERFACE_ATTR_LOOPBACK_PACKET_ACTION, attr_count, attr_list);
+    if (attr_loopback != NULL && is_ip_nbr_active())
+    {
+        vpp_apply_loopback_action(hw_ifname, attr_loopback->value.s32);
+    }
+
     return SAI_STATUS_SUCCESS;
 }
 
@@ -1032,10 +1041,21 @@ uint32_t SwitchVpp::find_new_bond_id()
     bool found_new_bond_id = false;
     while (std::getline(iss, line)) {
         std::string portchannel_name = line.substr(0, line.find('\n'));
-        bond_id = std::stoi(portchannel_name.substr(strlen(PORTCHANNEL_PREFIX)));
+        // A base PortChannel is named "PortChannel<id>". Sub-interfaces ("PortChannelX.Y"
+        // or the short "PoX.Y" form) and any malformed entry must not be mistaken for a new
+        // bond: a '.' or non-digit right after the prefix parses to a bogus id (often 0 via
+        // the safe-stoi fallback), which previously got assigned to a real LAG and left its
+        // RIF/neighbor on the wrong BondEthernet.
+        std::string suffix = portchannel_name.substr(strlen(PORTCHANNEL_PREFIX));
+        if (suffix.empty() || suffix[0] < '0' || suffix[0] > '9' ||
+            suffix.find('.') != std::string::npos || suffix.find('@') != std::string::npos) {
+            SWSS_LOG_DEBUG("Skipping non-base PortChannel entry for bond id: %s", portchannel_name.c_str());
+            continue;
+        }
+        bond_id = (uint32_t) vpp_safe_stoi(suffix, "find_new_bond_id");
 
         if (existing_bond_ids.find(bond_id) == existing_bond_ids.end()) {
-            SWSS_LOG_NOTICE("Found new bond id from PortChannel name: %d", bond_id);
+            SWSS_LOG_NOTICE("Found new bond id from PortChannel name: %u", bond_id);
             found_new_bond_id = true;
             break;
         }

vslib/vpp/SwitchVppNbr.cpp

@@ -31,37 +31,19 @@ sai_status_t SwitchVpp::addRemoveIpNbr(
 
     sai_deserialize_neighbor_entry(serializedObjectId, nbr_entry);
 
-    attr.id = SAI_ROUTER_INTERFACE_ATTR_PORT_ID;
-
-    CHECK_STATUS(get(SAI_OBJECT_TYPE_ROUTER_INTERFACE, nbr_entry.rif_id, 1, &attr));
-
-    auto port_obj_type = objectTypeQuery(attr.value.oid);
-    if (port_obj_type != SAI_OBJECT_TYPE_PORT && port_obj_type != SAI_OBJECT_TYPE_LAG)
-    {
-        return SAI_STATUS_SUCCESS;
-    }
-    auto port_oid = attr.value.oid;
-
-    attr.id = SAI_ROUTER_INTERFACE_ATTR_TYPE;
-
-    CHECK_STATUS(get(SAI_OBJECT_TYPE_ROUTER_INTERFACE, nbr_entry.rif_id, 1, &attr));
-    if (attr.value.s32 != SAI_ROUTER_INTERFACE_TYPE_SUB_PORT &&
-        attr.value.s32 != SAI_ROUTER_INTERFACE_TYPE_PORT)
+    // Resolve the VPP interface backing this RIF. vpp_get_rif_hwif_name handles
+    // PORT, LAG, SUB_PORT (hwif.vlan) and VLAN (bvi<vlan-id>) RIFs. The previous
+    // logic only accepted a PORT/LAG PORT_ID with PORT/SUB_PORT type, so VLAN RIF
+    // neighbors were silently skipped and never programmed into VPP -- the DUT then
+    // ARPed instead of forwarding looped traffic on a VLAN RIF.
+    std::string hwif_name;
+    if (vpp_get_rif_hwif_name(nbr_entry.rif_id, hwif_name) == false)
     {
-        SWSS_LOG_NOTICE("Skipping neighbor add for attr type %d", attr.value.s32);
-
+        SWSS_LOG_NOTICE("Skipping neighbor add: no VPP hwif for rif %s",
+                sai_serialize_object_id(nbr_entry.rif_id).c_str());
         return SAI_STATUS_SUCCESS;
     }
 
-    uint16_t vlan_id = 0;
-    if (attr.value.s32 == SAI_ROUTER_INTERFACE_TYPE_SUB_PORT)
-    {
-        attr.id = SAI_ROUTER_INTERFACE_ATTR_OUTER_VLAN_ID;
-
-        CHECK_STATUS(get(SAI_OBJECT_TYPE_ROUTER_INTERFACE, nbr_entry.rif_id, 1, &attr));
-        vlan_id = attr.value.u16;
-    }
-
     sai_mac_t nbr_mac;
     bool no_mac = true;
 
@@ -95,14 +77,6 @@ sai_status_t SwitchVpp::addRemoveIpNbr(
         return SAI_STATUS_FAILURE;
     }
 
-    std::string hwif_name;
-    bool found = vpp_get_hwif_name(port_oid, vlan_id, hwif_name);
-    if (found == false)
-    {
-        SWSS_LOG_ERROR("hw interface for port/lag id %s not found", serializedObjectId.c_str());
-        return SAI_STATUS_FAILURE;
-    }
-
     const char *vpp_ifname = hwif_name.c_str();
     init_vpp_client();