SAI icon indicating copy to clipboard operation
SAI copied to clipboard

[SAI-PTF][Defect][BFN] Default allow_mac_move value doesn't consistent with SAI definition

Open Gfrom2016 opened this issue 3 years ago • 1 comments

Summary

For static FDB entry, the default value of allow_mac_move attribute is false according to spec.

Details

In saifdb.h header file, default value of SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE is false for static FDB entry.

    /**
     * @brief Specifies whether a MAC move is allowed
     * When MAC_MOVE is explicitly disabled for a static MAC entry via this
     * attribute, the trap introduced in #696 would also not be generated.
     *
     * @type bool
     * @flags CREATE_AND_SET
     * @default false
     * @validonly SAI_FDB_ENTRY_ATTR_TYPE == SAI_FDB_ENTRY_TYPE_STATIC
     */
    SAI_FDB_ENTRY_ATTR_ALLOW_MAC_MOVE,

When we ran ptf test cases across different platforms, we found that PTF Test case test agsinst the configuration of allow_mac_move is true when calling sai_thrift_create_fdb_entry(), which isn't consistent with the SAI definition.

Take nativeVlanTest in saivlan.L2VlanTest for example, static FDB entry for mac address 00:33:33:33:33:33 is bind to port24 originally in setUp() without setting allow_mac_move true or false.

class L2VlanTest(SaiHelper):
    """
    The class runs VLAN test cases
    """

    def setUp(self):
        super(L2VlanTest, self).setUp()
        ...
        self.mac2 = '00:33:33:33:33:33:33'
        self.fdb_entry24 = sai_thrift_fdb_entry_t(
            switch_id=self.switch_id, mac_address=self.mac2, bv_id=self.vlan10)
        sai_thrift_create_fdb_entry(
            self.client,
            self.fdb_entry24,
            type=SAI_FDB_ENTRY_TYPE_STATIC,
            bridge_port_id=self.port24_bp,
            packet_action=mac_action)

When the code executes to send_packet(self, self.dev_port1, tag_pkt1) in the following code, different platforms have different results.

tag_pkt1 = simple_udp_packet(eth_dst='00:11:11:11:11:11',
                                eth_src='00:33:33:33:33:33',
                                dl_vlan_enable=True,
                                vlan_vid=10,
                                ip_ttl=64,
                                pktlen=104)
untag_pkt1 = simple_udp_packet(eth_dst='00:11:11:11:11:11',
                                eth_src='00:33:33:33:33:33',
                                ip_ttl=64,
                                pktlen=100)

print("Tx tag packet on trunk port %d -> access port %d" % (
    self.dev_port1, self.dev_port0))
send_packet(self, self.dev_port1, tag_pkt1)
verify_packet(self, untag_pkt1, self.dev_port0)

Without explicitly setting allow_mac_move to true(it should be false by default), static FDB entry of mac: 00:33:33:33:33:33 may cannot move from port24 to port1, due to its default configuration of allow_mac_move mismatching to spec, causing test case to failed.

Error message

image

Proposal

The code change in the sample PR is just a workaround, this issue should be fixed in barefoot environment.

Sample Code or Pull Requests for the proposal

#1601

Gfrom2016 avatar Sep 20 '22 05:09 Gfrom2016

@ravi861 could you help check this issue

richardyu-ms avatar Sep 26 '22 13:09 richardyu-ms