FDB notification for static entries

[mdivyamohan]

Hi,

I wanted to understand what is the expected behaviour from ASIC's SAI implementation related to FDB notification handling for static entries.

InredisPutFdbEntryToAsicView() of NotificationProcessor.cpp, entry type is assumed to be 'dynamic'.

    if (fdb->event_type == SAI_FDB_EVENT_LEARNED || fdb->event_type == SAI_FDB_EVENT_MOVE)
    {
        // currently we need to add type manually since fdb event don't contain type
        sai_attribute_t attr;

        attr.id = SAI_FDB_ENTRY_ATTR_TYPE;
        attr.value.s32 = SAI_FDB_ENTRY_TYPE_DYNAMIC;

Does this mean that when a static entry is added or deleted, SAI implementation is expected not to call fdb event callback?

[kcudnik]

previously that case was handling only LEARNED entries which were dynamic by default, and MOVE was added here https://github.com/Azure/sonic-sairedis/pull/670, i'm assuming that FDB static entries will not generate LEARNED/AGED notification, but probably (not sure here) can generate MOVE notification when something will be changed and then this SAI_FDB_ENTRY_TYPE_DYNAMIC should be STATIC

but what exact behavior should be here, i not 100% sure, @lguohan should you help here to brighten situation ?

@mdivyamohan did you faced any issues related to this ? if so what scenario ?

[mdivyamohan]

@kcudnik, this is background information:

For the ASIC that I am using, the SDK is generating generating LEARNED/AGED notifications for static FDB entries as well. i.e SDK notifications are generated when static entry is added to/removed from ASIC, and also when hit flags are updated.

Now in SAI implementation, I am trying to understand what would be the correct way - (1) or (2).

(1) Convert all SDK notifications (irrespective of whether entry is static or dynamic) to fdb event and send event to syncd.

This causes the problem with static entries, that syncd updates the entry to SAI_FDB_ENTRY_TYPE_DYNAMIC in ASIC DB. i.e with 'show mac', the static entries will be displayed as dynamic.

To fix this problem,

• SAI implementation should set SAI_FDB_ENTRY_ATTR_TYPE in event notification.
• The above pasted syncd code needs update - it should read SAI_FDB_ENTRY_ATTR_TYPE from notification, rather than always assuming 'SAI_FDB_ENTRY_TYPE_DYNAMIC'.

(2) Assume the expectation is that fdb events should be sent to syncd only for dynamic FDB entries. If this is how other platforms are behaving, the above syncd code would work just fine, if I skip sending notifications for static FDB entries to syncd.

[kcudnik]

When code was written then assumption was like in 2nd you described, because it seems redundant to send notification when user creates explicitly static entries, compared to dynamic entries when they can be created/remove at any time, hence notification is required. This approach seems logical to me, and so far platforms that im aware like mlnx or brcm, follow the same (although i don't remember if sonic is creating any static entries, or just deal with dynamic one)

I think this is a topic for SAI OCP weekly meeting (thursday), will you be able to create SAI issue related to this one in SAI repo, and perhaps join OCP meeting and describe the issue ?

[mdivyamohan]

@kcudnik I have opened a case in OCP SAI - https://github.com/opencomputeproject/SAI/issues/1353