[ovs-dev] [PATCH] ofp-parse: Support "igmp" keyword in flows.

[Ilya Maximets]

On 11/3/20 5:46 PM, Ben Pfaff wrote:
> On Tue, Nov 03, 2020 at 03:53:51AM +0100, Ilya Maximets wrote:
>> On 11/3/20 12:28 AM, Ben Pfaff wrote:
>>> match_format() prints out "igmp" for IGMP flows, but
>>> ofp_parse_protocol() didn't accept it, which meant that OVS would print
>>> out a flow that it wouldn't re-parse.  This fixes the problem and adds
>>> a test.
>>
>> I'm a bit confused.  IIUC, matching on igmp is not supported by any
>> OF version or by any extensions, i.e. we could match by 'ip,nw_proto=2',
>> but there is no special OF header for "igmp" or "igmp_type" or "igmp_code".
>> While it seems easy to add parsing of "igmp" itself, its fields ("igmp_type"
>> and "igmp_code") could appear in the output too and parsing of these fields
>> will, I guess, require a new OF extension.  Is it correct?  If so, maybe
>> it's better to remove "igmp*" printing code from the match_format() instead?
> 
> I hadn't thought about that.  I was just thinking of this as a short
> form of "ip,nw_proto=2".
> 
> It looks like OVS's support for IGMP matching is inconsistent.
> flow_extract() pulls the igmp_type and igmp_code values into tp_src and
> tp_dst.  The kernel datapath doesn't do that, though, and there's no
> support for the fields at the OpenFlow level.  I guess it has never been
> important enough.

>From my understanding, for now we only need support inside OVS userspace
for multicast snooping.  So, OVS just instructs datapath that all igmp
(ip,nw_proto=2) packets should go to userspace, so they could be specially
processed.  And this is just a configuration of the bridge, so no need
to have support in OpenFlow.  I think, the same we have for LLDP:  OVS
instructs datapath that all 0x88cc packets should go to userspace and
userspace handles these packets adjusting the configuration of ports and
bridges.  In this case we don't really need any special support from the
datapath and this also works without involving the OF level as these
packets are consumed by OVS itself and not passed through OF pipelines.

> 
> OK, I won't complain either way then.

I think, it's better to just stop printing word 'igmp' in flow dumps to
avoid confusion since OF doesn't support igmp.  We're not printing anything
like this for lldp or some other types of packets anyway.

Flavio, you were the original author of this mcast snooping feature, what
do you think?

Best regards, Ilya Maximets.