Skip to content

Add VLAN-aware MLD Snooping support - #2326

Open
AnandhiDhanabalan wants to merge 1 commit into
opencomputeproject:masterfrom
AnandhiDhanabalan:mld_snooping
Open

Add VLAN-aware MLD Snooping support#2326
AnandhiDhanabalan wants to merge 1 commit into
opencomputeproject:masterfrom
AnandhiDhanabalan:mld_snooping

Conversation

@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor

No description provided.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@kcudnik

kcudnik commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor Author

@kcudnik , This PR also has enum alias added. The failure seems to be related to this. Can you help fix it ?

  SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE,

    /** @ignore - for backward compatibility */
    SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE = SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE,

Signed-off-by: Anandhi Dhanabalan <adhanabalan@marvell.com>
@kcudnik

kcudnik commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

squash commits and force push

@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor Author

@tjchadaga could you please assign a reviewer for this PR

@kcudnik

kcudnik commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@tjchadaga tjchadaga added the reviewed PR is discussed in SAI Meeting label Aug 12, 2026
@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor Author

@kcudnik, the failure appears to be due to the enum alias usage. Should I take any action on this, or will you be handling it? Please advice.

**2026-08-07T17:46:29.4352920Z WARNING: Both enums have the same value SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE and SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE = 0x00000014**
2026-08-07T17:46:29.4353576Z processing commit 9611265
2026-08-07T17:46:29.4353918Z ERROR: please correct all 0 error(s) and all 1 warnings before continue
2026-08-07T17:46:29.4354361Z make: *** [Makefile:89: all] Error 1
2026-08-07T17:46:29.4367116Z 
2026-08-07T17:46:29.4409303Z ##[error]Bash exited with code '2'.
2026-08-07T17:46:29.4538788Z ##[section]Finishing: Metadata check

@kcudnik

kcudnik commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

i think problem is taht SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE is ignored, which is legitimate enum in previous commits, make @ignore enum you are adding, and assign it by SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE

 /** @ignore - for backward compatibility */
    SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE = SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE

you will need to squash commits and force push, check locally before you do that ,

@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor Author

@kcudnik, just to confirm my understanding: with the following definition, SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE is the enum being marked as @ignore, correct?

/** @ignore - for backward compatibility */
   SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE = SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE 

In my case, SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE is the existing attribute that I want to deprecate/ignore, while SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE is the new attribute. My understanding from the existing @ignore usage is that the @ignore annotation should be placed on the enum that needs to be deprecated. Could you please confirm the intended definition?

@kcudnik

kcudnik commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

no, @ignore is placed on attribute that have the same enum ID as other exdisting attribute, if you have both wih the same value, none of them can be depreacated

@AnandhiDhanabalan

Copy link
Copy Markdown
Contributor Author

@kcudnik , I’m seeing the same meta error irrespective of how the enum is defined. Capturing the error below:

WARNING: Both enums have the same value SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE and SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE = 0x00000014
ERROR: please correct all 0 error(s) and all 1 warnings before continue
make: *** [Makefile:89: all] Error 1

Tried both variants:

    /**
     * @brief IGMP Snooping enable or disable control for VLAN
     *
     * IGMP Snooping enable control for VLAN. Default is
     * disabled
     *
     * @type bool
     * @flags CREATE_AND_SET
     * @default false
     */
    SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE,

    /** @ignore - for backward compatibility */
    SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE = SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE,

and

    /**
     * @brief IGMP Snooping enable or disable control for VLAN
     *
     * IGMP Snooping enable control for VLAN. Default is
     * disabled
     *
     * @type bool
     * @flags CREATE_AND_SET
     * @default false
     */
    SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE,

    /** @ignore - for backward compatibility */
    SAI_VLAN_ATTR_IGMP_SNOOPING_ENABLE = SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE,

In both cases, meta flags duplicate enum values.

I also noticed similar @ignore usage is already present (e.g. in sainexthopgroup.h):

typedef enum _sai_next_hop_group_type_t
{
    /** Next hop group is ECMP, with a dynamic number of members, unordered */
    SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP,

    /** @ignore - for backward compatibility */
    SAI_NEXT_HOP_GROUP_TYPE_ECMP = SAI_NEXT_HOP_GROUP_TYPE_DYNAMIC_UNORDERED_ECMP,

Could you please confirm:

  • whether enum aliasing like this is still allowed for new additions, or
  • if meta checks have been tightened and we should avoid introducing aliases?

If aliases are not allowed, I’ll retain only SAI_VLAN_ATTR_CUSTOM_IGMP_SNOOPING_ENABLE and avoid adding the new name. Please advise what’s the preferred approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewed PR is discussed in SAI Meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants