[zebra_fpm]: FIX vrf master device index can't be larger than 255

[tylerlinp]

The existing Frr suite have a bug. When master device index is larger than 255 , index will be truncated to uchar. For vrf support we set master index to another data structure following netlink message definition.

[pavel-shirshov]

Is your patch is enough?
I see the FRR has more fixes for the bug: FRRouting/frr@6dfcd75

[tylerlinp]

Is your patch is enough?
I see the FRR has more fixes for the bug: FRRouting/frr@6dfcd75

Yes, enough. Now vrf implementation depends on it.
In fact, FRR commit fills the real table_id, it is more consistent with netlink definitions.
But, my commit & origin fills the vrf_id to rtm_table, it is a fake table_id, vrf_id in FRR is just vrf ifindex in kernel.
At the beginning, I had modified it just like FRR commit except RT_TABLE_UNSPEC/RT_TABLE_COMPAT(linux kernel fills RT_TABLE_COMPAT if table >= 256, libnl doesn't check it). But then I found it is more convenient(in orchs no table_id) in SONiC if keep ifindex, so I reserve it.
The route table based table_id in linux kernel and based vrf_id in FRR. Actually linux can create multiple vrf device in same table. Maybe to maintain uniqueness vrf_id has certain rationality.

[preetham-singh]

rtm_table is set to RT_TABLE_COMPAT here for programming routes to FPMSYNCD.
Shouldnt same be done for programming route to kernel? I see that RT_TABLE_UNSPEC is used while programming to kernel. Would this cause inconsistency since these values are different and it might go to different tables when application has vrf in u8?
Also, as per FRR commit: FRRouting/frr#4365 , rtm_table is set to RT_TABLE_UNSPEC . If this can be consistent it would be helpful when this FRR fix is merged to SONiC.

[tylerlinp]

rtm_table is set to RT_TABLE_COMPAT here for programming routes to FPMSYNCD.
Shouldnt same be done for programming route to kernel? I see that RT_TABLE_UNSPEC is used while programming to kernel. Would this cause inconsistency since these values are different and it might go to different tables when application has vrf in u8?
Also, as per FRR commit: FRRouting/frr#4365 , rtm_table is set to RT_TABLE_UNSPEC . If this can be consistent it would be helpful when this FRR fix is merged to SONiC.

Using RT_TABLE_COMPAT is because I consulted how linux kernel fill this. (rt_fill_info in net/ipv4/route.c)
Like you said, FRR fill netlink to kernel using RT_TABLE_UNSPEC. (netlink_route_multipath in zebra/rt_netlink.c)
In fact it doesn't matter, rtm_table will be overwrite by u32 data when parsing message.
Like this:
rt_table = rtm->rtm_table;
if (tb[RTA_TABLE])
rt_table = RTA_DATA(tb[RTA_TABLE]));

To keep consistency in FRR, it would be RT_TABLE_UNSPEC. But now frr#4365 cannot be merged to SONiC directly, just as above comment. It wouldn't be a good job even if redesign fpmsyncd to translate table_id.

[pavel-shirshov]

Can we use this patch to incorporate for FRR master sources into branch 7.0?

[lguohan]

agree, it should be frr/7.0 branch.

[pavel-shirshov]

We can add patches to sonic-frr in two ways:
Either make a PR for https://github.com/FRRouting/frr/tree/stable/7.0
Or add a patch as a .patch file into https://github.com/Azure/sonic-buildimage/tree/master/src/sonic-frr/patch

[shine4chen]

@pavel-shirshov Sure, we will add a .patch file soon. At the same time we will submit a PR to FRR community.

[prsunny]

Closing this in favor of sonic-net/sonic-buildimage#3280