Ilya Maximets <[email protected]> writes:

> On 9/15/21 21:04, Aaron Conole wrote:
>> When OVS starts with default settings, it will have no existing datapath
>> flows configured, and it will not explicitly handle IGMP packets.  This
>> means that all traffic will hit the datapath, and then follow the
>> xlate_normal processing path.
>> 
>> Unfortunately for some users of IGMP-aware applications (such as
>> 'keepalived'), IGMP packets will arrive, go through processing and a
>> default flow like following will be installed:
>> 
>>    recirc_id(0),in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), 
>> actions:userspace(pid=xxxxxxx,slow_path(match))
>> 
>> This is a very broad match - and will force all IPv4 traffic to userspace.
>> 
>> To combat this, force the wildcard initialization to always include an
>> IGMP protocol match.  An existing IGMP check is only run when multicast
>> snooping is configured.  Now we will always run the check during wildcard
>> init.  A unit test is added that works for kernel and userspace datapaths.
>> 
>> Reported-by: Lorenzo Bianconi <[email protected]>
>> Reported-by: Mohamed Mahmoud <[email protected]>
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2002888
>> Signed-off-by: Aaron Conole <[email protected]>
>> ---
>>  ofproto/ofproto-dpif-xlate.c |  8 ++++++++
>>  tests/system-traffic.at      | 28 ++++++++++++++++++++++++++++
>>  2 files changed, 36 insertions(+)
>> 
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index 8723cb4e85..dc3971cdf9 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -7381,6 +7381,14 @@ xlate_wc_init(struct xlate_ctx *ctx)
>>          WC_MASK_FIELD_MASK(ctx->wc, nw_frag, FLOW_NW_FRAG_MASK);
>>      }
>>  
>> +    /* Always check for igmp type in the packet.  This will ensure that
>> +     * the igmp nw type will properly be set as a match field.  */
>> +    if (get_dl_type(&ctx->xin->flow) == htons(ETH_TYPE_IP)) {
>> +        if (ctx->xin->flow.nw_proto == IPPROTO_IGMP && ctx->wc) {
>> +            WC_MASK_FIELD(ctx->wc, nw_proto);
>> +        }
>> +    }
>> +
>>      if (ctx->xbridge->support.odp.recirc) {
>>          /* Always exactly match recirc_id when datapath supports
>>           * recirculation.  */
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index de9108ac20..e0836839d6 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>
> Hi, Aaron.
> I'm not going deep into the issue and the actual change for now,
> but it seems to be very generic.  Can we have a test in a main
> testsuite instead of a system one?  e.g. in odproto-dpif.at.
> Packet can be injected with 'ovs-appctl netdev-dummy/receive' in
> this case.

Yes.

> Best regards, Ilya Maximets.
>
>> @@ -6147,6 +6147,34 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep
>> table=2, | OFPROTO_CLEAR_DURATION_IDLE
>>  OVS_TRAFFIC_VSWITCHD_STOP
>>  AT_CLEANUP
>>  
>> +AT_BANNER([IGMP])
>> +
>> +AT_SETUP([IGMP - VRRP VSS padded])
>> +
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +ADD_NAMESPACES(at_ns0, at_ns1)
>> +
>> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01")
>> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02")
>> +
>> +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 
>> dnl
>> +f0 00 00 01 01 01 08 00 46 c0 00 28 00 00 40 00 01 02 d3 49 45 65 eb 4a e0 
>> dnl
>> +00 00 16 94 04 00 00 22 00 f9 02 00 00 00 01 04 00 00 00 e0 00 00 fb 00 00 
>> dnl
>> +00 00 00 00 > /dev/null])
>> +
>> +AT_CHECK([ovs-appctl dpctl/dump-flows | grep -e .*ipv4 | dnl
>> +          sed -e 's/ packets:[[0-9]]*,//' -e 's/ bytes:[[0-9]]*,//' dnl
>> +              -e 's/ used:[[a-zA-Z0-9]]*,//' -e 's/pid=[[0-9]]*,//' dnl
>> +              -e 's/,packet_type(ns=[[0-9]]*,id=[[0-9]]*),/,eth(),/' dnl
>> +              -e 's/actions:drop/actions:userspace(slow_path(match))/'],
>> +                     [0], [dnl
>> +recirc_id(0),in_port(2),eth(),eth_type(0x0800),ipv4(proto=2,frag=no),
>> actions:userspace(slow_path(match))
>> +])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>> +
>>  AT_BANNER([802.1ad])
>>  
>>  AT_SETUP([802.1ad - vlan_limit])
>> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to