[draft] Saithriftv2 Makefile for SAIVPP UT

debian/python-saithrift.install

@@ -1 +1,2 @@
-debian/usr/local/lib/python2.7/site-packages/* /usr/lib/python2.7/dist-packages/
+#compatiable with bookworm python 3.11 environment and build with python3
+debian/usr/local/local/lib/python3*/dist-packages/* /usr/lib/python3/dist-packages/

meta/templates/sai_rpc_server_helper_functions.tt

@@ -31,7 +31,8 @@
 }
             [%- END -%]
             [%- FOREACH struct IN apis.$api.structs %]
-
+                [%- NEXT IF struct.short_name == 'neighbor_entry' %]
+                [%- NEXT IF struct.short_name == 'route_entry' %]
 
                 [%- PROCESS struct_helper_function_header %] {
 
@@ -135,8 +136,57 @@ void sai_thrift_parse_[% struct.short_name %](const [% struct.thrift_name %] &th
 [%- ######################################################################## -%]
 
 [%- BLOCK special_helper_functions -%]
+extern sai_object_id_t gSwitchId;
+extern sai_object_id_t switch_id;
+
 void sai_thrift_parse_buffer(const std::string &thrift_buffer,
                              void *buffer) {
   /* not supported yet */
 }
+
+void sai_thrift_parse_neighbor_entry(const sai_thrift_neighbor_entry_t &thrift_neighbor_entry,
+                                     sai_neighbor_entry_t *neighbor_entry)
+{
+    if (neighbor_entry == NULL) {
+        return;
+    }
+
+    // OCP sai_test often omits switch_id on neighbor_entry; fall back to the
+    // RPC-global switch_id (set by sai_thrift_create_switch) then gSwitchId.
+    if (thrift_neighbor_entry.switch_id != 0) {
+        neighbor_entry->switch_id = (sai_object_id_t)thrift_neighbor_entry.switch_id;
+    } else if (switch_id != SAI_NULL_OBJECT_ID) {
+        neighbor_entry->switch_id = switch_id;
+    } else if (gSwitchId != SAI_NULL_OBJECT_ID) {
+        neighbor_entry->switch_id = gSwitchId;
+    } else {
+        neighbor_entry->switch_id = SAI_NULL_OBJECT_ID;
+    }
+
+    neighbor_entry->rif_id = (sai_object_id_t)thrift_neighbor_entry.rif_id;
+    sai_thrift_ip_address_t_parse(thrift_neighbor_entry.ip_address,
+                                  &neighbor_entry->ip_address);
+}
+
+void sai_thrift_parse_route_entry(const sai_thrift_route_entry_t &thrift_route_entry,
+                                  sai_route_entry_t *route_entry)
+{
+    if (route_entry == NULL) {
+        return;
+    }
+
+    if (thrift_route_entry.switch_id != 0) {
+        route_entry->switch_id = (sai_object_id_t)thrift_route_entry.switch_id;
+    } else if (switch_id != SAI_NULL_OBJECT_ID) {
+        route_entry->switch_id = switch_id;
+    } else if (gSwitchId != SAI_NULL_OBJECT_ID) {
+        route_entry->switch_id = gSwitchId;
+    } else {
+        route_entry->switch_id = SAI_NULL_OBJECT_ID;
+    }
+
+    route_entry->vr_id = (sai_object_id_t)thrift_route_entry.vr_id;
+    sai_thrift_ip_prefix_t_parse(thrift_route_entry.destination,
+                                 &route_entry->destination);
+}
 [% END -%]

test/sai_test/config/port_configer.py

@@ -18,6 +18,7 @@
 #
 #
 
+import os
 from collections import OrderedDict
 from ptf import config
 from sai_utils import *  # pylint: disable=wildcard-import; lgtm[py/polluting-import]
@@ -120,7 +121,6 @@ def create_bridge_ports(self, bridge_id, port_list: List['Port']):
         for index, item in enumerate(port_list):
             port_bp = sai_thrift_create_bridge_port(
                 self.client,
-                bridge_id=bridge_id,
                 port_id=item.oid,
                 type=SAI_BRIDGE_PORT_TYPE_PORT,
                 admin_state=True)
@@ -498,7 +498,22 @@ def turn_up_and_get_checked_ports(self, port_list: List['Port']):
         '''
 
         # For brcm devices, need to init and setup the ports at once after start the switch.
-        retries = 10
+        #
+        # Waiting strategy is configurable via environment, defaulting to the original
+        # behavior (retries=2, 1s poll interval). The original loop waited PER PORT
+        # serially (retries x interval seconds for each of N ports), which on a large
+        # port count where oper-status is slow to settle costs N * retries * interval
+        # seconds (e.g. 32 ports x 2 x 1s ~= 64s). The shared-wait path below issues
+        # admin-up to every port once, then polls ALL ports together for at most
+        # (retries x interval) seconds total, re-asserting admin-up on stragglers each
+        # round. Semantics are preserved: every port is set admin-up, oper-status is
+        # polled, and down_port_list / the returned list are identical; only the total
+        # wall-clock wait changes. Real-HW/ASIC consumers that do not set these env
+        # vars keep the exact original timing.
+        retries = int(os.environ.get('SAI_PORT_UP_RETRIES', '2'))
+        poll_interval = float(os.environ.get('SAI_PORT_UP_POLL_INTERVAL', '1'))
+        # Opt-in: poll all ports together in one bounded loop instead of per-port.
+        shared_wait = os.environ.get('SAI_PORT_UP_SHARED_WAIT', '0') == '1'
         down_port_list = []
         test_port_list:List[Port] = []
 
@@ -514,29 +529,49 @@ def turn_up_and_get_checked_ports(self, port_list: List['Port']):
                         port_oid=port.oid,
                         admin_state=True)
 
-        for index, port in enumerate(test_port_list):
-            port_attr = sai_thrift_get_port_attribute(
-                self.client, port.oid, oper_status=True)
-            print("Turn up port {}".format(index))
-            port_up = True
-            if port_attr['oper_status'] != SAI_PORT_OPER_STATUS_UP:
-                port_up = False
-                for num_of_tries in range(retries):
+        if shared_wait:
+            # Single bounded wait shared across all ports.
+            pending = list(enumerate(test_port_list))
+            for num_of_tries in range(retries + 1):
+                still_down = []
+                for index, port in pending:
                     port_attr = sai_thrift_get_port_attribute(
                         self.client, port.oid, oper_status=True)
-                    if port_attr['oper_status'] == SAI_PORT_OPER_STATUS_UP:
-                        port_up = True
-                        break
-                    time.sleep(3)
-                    self.log_port_state(port, index)
-                    print("port {} , local index {} id {} is not up, status: {}. Retry. Reset Admin State.".format(
-                        index, port.port_index, port.oid, port_attr['oper_status']))
-                    sai_thrift_set_port_attribute(
-                        self.client,
-                        port_oid=port.oid,
-                        admin_state=True)
-            if not port_up:
-                down_port_list.append(index)
+                    if port_attr['oper_status'] != SAI_PORT_OPER_STATUS_UP:
+                        still_down.append((index, port))
+                pending = still_down
+                if not pending:
+                    break
+                if num_of_tries < retries:
+                    time.sleep(poll_interval)
+                    for index, port in pending:
+                        sai_thrift_set_port_attribute(
+                            self.client, port_oid=port.oid, admin_state=True)
+            down_port_list = [index for index, _ in pending]
+        else:
+            for index, port in enumerate(test_port_list):
+                port_attr = sai_thrift_get_port_attribute(
+                    self.client, port.oid, oper_status=True)
+                print("Turn up port {}".format(index))
+                port_up = True
+                if port_attr['oper_status'] != SAI_PORT_OPER_STATUS_UP:
+                    port_up = False
+                    for num_of_tries in range(retries):
+                        port_attr = sai_thrift_get_port_attribute(
+                            self.client, port.oid, oper_status=True)
+                        if port_attr['oper_status'] == SAI_PORT_OPER_STATUS_UP:
+                            port_up = True
+                            break
+                        time.sleep(poll_interval)
+                        self.log_port_state(port, index)
+                        print("port {} , local index {} id {} is not up, status: {}. Retry. Reset Admin State.".format(
+                            index, port.port_index, port.oid, port_attr['oper_status']))
+                        sai_thrift_set_port_attribute(
+                            self.client,
+                            port_oid=port.oid,
+                            admin_state=True)
+                if not port_up:
+                    down_port_list.append(index)
         if down_port_list:
             print("Ports {} are  down after retries.".format(down_port_list))
         return test_port_list

test/sai_test/config/route_configer.py

@@ -290,15 +290,17 @@ def create_default_v4_v6_route_entry(self):
                                             addr=sai_thrift_ip_addr_t(
                                                 ip6=DEFAULT_IP_V6_PREFIX),
                                             mask=sai_thrift_ip_addr_t(ip6=DEFAULT_IP_V6_PREFIX))
-        self.test_obj.dut.default_ipv6_route_entry = sai_thrift_route_entry_t(vr_id=self.test_obj.dut.default_vrf,
+        self.test_obj.dut.default_ipv6_route_entry = sai_thrift_route_entry_t(switch_id=self.test_obj.dut.switch_id,
+                                                                              vr_id=self.test_obj.dut.default_vrf,
                                                                               destination=v6_default)
         status = sai_thrift_create_route_entry(
             self.client,
             route_entry=self.test_obj.dut.default_ipv6_route_entry,
             packet_action=SAI_PACKET_ACTION_DROP)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
 
-        self.test_obj.dut.default_ipv4_route_entry = sai_thrift_route_entry_t(vr_id=self.test_obj.dut.default_vrf,
+        self.test_obj.dut.default_ipv4_route_entry = sai_thrift_route_entry_t(switch_id=self.test_obj.dut.switch_id,
+                                                                              vr_id=self.test_obj.dut.default_vrf,
                                                                               destination=sai_ipprefix(DEFAULT_IP_V4_PREFIX))
         status = sai_thrift_create_route_entry(
             self.client,
@@ -323,22 +325,22 @@ def create_route_by_rif(
         vr_id = self.choice_virtual_route(virtual_router)
         if dest_device.ip_prefix:
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
         else:
             # destination cannot use sai_ipaddress
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev4, next_hop_id=rif)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
 
         if dest_device.ip_prefix_v6:
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
         else:
             # destination cannot use sai_ipaddress
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev6, next_hop_id=rif)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
@@ -366,22 +368,22 @@ def create_route_by_nexthop(
         vr_id = self.choice_virtual_route(virtual_router)
         if dest_device.ip_prefix:
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
         else:
             # destination cannot use sai_ipaddress
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev4, next_hop_id=nexthopv4.oid)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
 
         if dest_device.ip_prefix_v6:
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
         else:
             # destination cannot use sai_ipaddress
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev6, next_hop_id=nexthopv6.oid)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
@@ -406,22 +408,22 @@ def create_route_by_nexthop_group(
         vr_id = self.choice_virtual_route(virtual_router)
         if dest_device.ip_prefix:
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/'+dest_device.ip_prefix))
         else:
             # destination cannot use sai_ipaddress
             net_routev4 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv4+'/32'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev4, next_hop_id=nexthop_groupv4.nhp_grp_id)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
 
         if dest_device.ip_prefix_v6:
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/'+dest_device.ip_prefix_v6))
         else:
             # destination cannot use sai_ipaddress
             net_routev6 = sai_thrift_route_entry_t(
-                vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
+                switch_id=self.test_obj.dut.switch_id, vr_id=vr_id, destination=sai_ipprefix(dest_device.ipv6+'/128'))
         status = sai_thrift_create_route_entry(
             self.client, net_routev6, next_hop_id=nexthop_groupv6.nhp_grp_id)
         self.test_obj.assertEqual(status, SAI_STATUS_SUCCESS)
@@ -447,6 +449,7 @@ def create_neighbor_by_rif(self, nexthop_device: Device, rif, no_host=True):
         """
         if nexthop_device.ipv4:
             nbr_entry_v4 = sai_thrift_neighbor_entry_t(
+                switch_id=self.test_obj.dut.switch_id,
                 rif_id=rif,
                 ip_address=sai_ipaddress(nexthop_device.ipv4))
             status = sai_thrift_create_neighbor_entry(
@@ -460,6 +463,7 @@ def create_neighbor_by_rif(self, nexthop_device: Device, rif, no_host=True):
 
         if nexthop_device.ipv6:
             nbr_entry_v6 = sai_thrift_neighbor_entry_t(
+                switch_id=self.test_obj.dut.switch_id,
                 rif_id=rif,
                 ip_address=sai_ipaddress(nexthop_device.ipv6))
             status = sai_thrift_create_neighbor_entry(
@@ -622,9 +626,13 @@ def create_nexthop_group_by_nexthops(self, nexthopv4_list: List[Nexthop], nextho
             nhp_grpv4_members.append(nhp_grpv4_member)
             nhp_grpv6_members.append(nhp_grpv6_member)
 
+        # Give each NHG its own copy of the member port-index list. The v4 and v6
+        # groups are mutated independently (remove/re-add member tests), so sharing
+        # one list object would let a v4 mutation corrupt the v6 group's port list
+        # (ValueError: x not in list) when both run in the same config group.
         member_port_indexs = [17, 18, 19, 20, 21, 22, 23, 24]
-        nhp_grpv4: NexthopGroup = NexthopGroup(nhop_groupv4_id, nhp_grpv4_members, member_port_indexs)
-        nhp_grpv6: NexthopGroup = NexthopGroup(nhop_groupv6_id, nhp_grpv6_members, member_port_indexs)
+        nhp_grpv4: NexthopGroup = NexthopGroup(nhop_groupv4_id, nhp_grpv4_members, list(member_port_indexs))
+        nhp_grpv6: NexthopGroup = NexthopGroup(nhop_groupv6_id, nhp_grpv6_members, list(member_port_indexs))
 
         self.test_obj.dut.nhp_grpv4_list.append(nhp_grpv4)
         self.test_obj.dut.nhp_grpv6_list.append(nhp_grpv6)

test/saithrift/src/switch_sai.thrift

@@ -198,8 +198,9 @@ struct sai_thrift_route_entry_t {
 }
 
 struct sai_thrift_neighbor_entry_t {
-    1: sai_thrift_object_id_t rif_id;
-    2: sai_thrift_ip_address_t ip_address;
+    1: sai_thrift_object_id_t switch_id;
+    2: sai_thrift_object_id_t rif_id;
+    3: sai_thrift_ip_address_t ip_address;
 }
 
 struct sai_thrift_attribute_list_t {

test/saithrift/src/switch_sai_rpc_server.cpp

@@ -241,7 +241,13 @@ class switch_sai_rpcHandler : virtual public switch_sai_rpcIf {
   }
 
   void sai_thrift_parse_neighbor_entry(const sai_thrift_neighbor_entry_t &thrift_neighbor_entry, sai_neighbor_entry_t *neighbor_entry) {
-      neighbor_entry->switch_id = gSwitchId;
+      // Python saithrift sends switch_id as field 1 (optional). Always fall back to
+      // gSwitchId when the client omits it (OCP sai_test often only passes rif_id).
+      if (thrift_neighbor_entry.switch_id != 0) {
+          neighbor_entry->switch_id = (sai_object_id_t) thrift_neighbor_entry.switch_id;
+      } else {
+          neighbor_entry->switch_id = gSwitchId;
+      }
       neighbor_entry->rif_id = (sai_object_id_t) thrift_neighbor_entry.rif_id;
       sai_thrift_parse_ip_address(thrift_neighbor_entry.ip_address, &neighbor_entry->ip_address);
   }

test/saithriftv2/Makefile

@@ -38,7 +38,10 @@ CTYPESGEN = /usr/local/bin/ctypesgen.py
 endif
 
 ifeq ($(platform),vs)
-LIBS = -lthrift -lpthread -lsaivs -lsaimeta -lsaimetadata -lzmq
+LIBS = -lthrift -lpthread -lsaivs -lsaimeta -lsaimetadata -lzmq -lswsscommon
+else ifeq ($(platform),vpp)
+LIBS = -lthrift -lpthread -lsaivs -lsaimeta -lsaimetadata -lzmq \
+       -lvlib -lvlibapi -lvppapiclient -lvlibmemoryclient -lvppinfra -lswsscommon
 else
 LIBS = -lthrift -lpthread -lsai -lsaimetadata
 endif

test/saithriftv2/convert_header.py

@@ -47,7 +47,7 @@ class sai_common_api(SAIEnum):
 import enum
 class SAIEnum(enum.IntEnum):
     def __str__(self):      
-        return super().__str__().split(\".\")[1]\n"""
+        return self.name\n"""
 ENUM_PREFIX = "enum__sai_"
 SAI_NAME  = "SAI"
 ENUM_TYPE = "= c_int"

test/saithriftv2/src/saiserver.cpp

@@ -21,6 +21,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include "sai_rpc.h"
+#include <swss/logger.h>
 
 #define UNREFERENCED_PARAMETER(P)   (P)
 
@@ -269,6 +270,9 @@ void handleInitScript(const std::string& initScript)
 int
 main(int argc, char* argv[])
 {
+    swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG);
+    swss::Logger::getInstance().swssOutputNotify("saiserver", "STDOUT");
+
     int rv = 0;
 
     auto options = handleCmdLine(argc, argv);