On 1/17/24 13:14, Eelco Chaudron wrote: > > > On 17 Jan 2024, at 12:16, Ilya Maximets wrote: > >> On 1/16/24 15:26, Eelco Chaudron wrote: >>> >>> >>> On 16 Jan 2024, at 15:20, Ilya Maximets wrote: >>> >>>> On 12/27/23 12:15, Mohammad Heib wrote: >>>>> Store igmp/mld protocol version into the >>>>> mcast_group internally, the multicast snooping feature >>>>> is used by many OVS consumers and those consumers heavily rely >>>>> on the OVS implementation to manage/deal with mcast groups, >>>>> some of those consumers also need to deal/expose the mcast protocol >>>>> to the end user for debuggability purposes. >>>>> >>>>> OVN for example needs to expose the protocol version to the end user >>>>> to match between the protocol version used in the OVN logical switches >>>>> and the uplink ports >>>>> >>>>> Therefore, instead of implementing this in each OVS consumer that needs >>>>> to deal mcast group protocol version which will be very complicated >>>>> implementation since it rely on the OVS code, saving the protocol to >>>>> the mdb inside OVS will give that consumer access to the protocol version >>>>> very easily. >>>>> >>>>> Signed-off-by: Mohammad Heib <[email protected]> >>>>> --- >>>>> v6: Rebase on top of current master. >>>>> Address comments from Eelco: >>>>> - hardcode MCAST_GROUP_IGMPV3 inside mcast_snooping_add_report >>>>> function. >>>>> --- >>>> >>>> Hi, Eelco and Simon, do you have any further comments on this patch set? >>>> i.e. did you have a chance to review this version? I see you both looked >>>> at the previous ones. >>>> >>>> I see some conversation here, but I'm not sure if it is resolved or not. >>>> >>>> (I didn't review the code myself and will likley not have much time before >>>> branching tomorrow.) >>> >>> My plan was to do it todays morning, but did not happen :) Will do it >>> before the end of the day. >> >> Thanks, Eelco. >> >> I see the changes you requested are mostly cosmetic. Will you be comfortable >> making them yourself while applying the set? Asking because I believe >> Mohammad is on PTO and will not be able to follow up with a new version >> before >> branching, i.e. the feature may miss the release. >> >> Maybe also add a small NEWS entry to the ovs-appctl section that mdb/show now >> provides multicast group protocol information. > > Sound good to me, let me prepare this. If you see any issues with my > suggestions let me know, else I’ll commit.
Your style/formatting suggestions LGTM. > > //Eelco > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
