On 5 Jul 2022, at 0:48, Ilya Maximets wrote:
> On 7/4/22 18:52, Eelco Chaudron wrote: >> Hi Abhiram, >> >> It’s been there since your patch was committed to the repo. >> I did a quick verification again, and it is: >> >> git checkout 7539b4e45 >> <edit test> >> git diff >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >> index 4a7fa49fc..b55d757bf 100644 >> --- a/tests/system-traffic.at >> +++ b/tests/system-traffic.at >> @@ -393,6 +393,9 @@ OVS_CHECK_GRE() >> OVS_CHECK_ERSPAN() >> >> OVS_TRAFFIC_VSWITCHD_START() >> + >> +ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true >> + >> ADD_BR([br-underlay]) >> >> make -j 32 check-kernel TESTSUITEFLAGS='-k "ping over erspan v1 tunnel" -v' >> ... >> ./system-traffic.at:417: ip netns exec at_ns0 sh << NS_EXEC_HEREDOC >> ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | grep "transmitted" | sed >> 's/time.*ms$/time 0ms/' >> NS_EXEC_HEREDOC >> ./system-traffic.at:423: ip netns exec at_ns0 sh << NS_EXEC_HEREDOC >> ping -s 1200 -i 0.3 -c 3 10.1.1.100 | grep "transmitted" | sed >> 's/time.*ms$/time 0ms/' >> NS_EXEC_HEREDOC >> --- - 2022-07-04 18:48:07.788952607 +0200 >> +++ >> /home/echaudron/Documents/Scratch/ovs/tests/system-kmod-testsuite.dir/at-groups/13/stdout >> 2022-07-04 18:48:07.785404896 +0200 >> @@ -1,2 +1,2 @@ >> -3 packets transmitted, 3 received, 0% packet loss, time 0ms >> +3 packets transmitted, 1 received, 66.6667% packet loss, time 0ms >> ... >> >> >> Can you please investigate? I know reverting the patch will not offload to >> TC, but it’s not working, so it’s better to not offload and work than to >> offload and not work ;) > > I sent some patches to resolve the issue: > https://patchwork.ozlabs.org/project/openvswitch/list/?series=307976&state=* > > It's not a problem of ERSPAN in particular, but a wider > problem with ignoring a lot of tunnel keys and flags. > > ERSPAN offload will probably work if no options are specified, > but I'm not sure how much sense it makes this way. Ilya, Your patches look good, however, the test cases for ERSPAN use ERSPAN-specific options, so they will fall back to kerne dp. Abhiram, so if you need a specific ERSPAN option with HW offload this will not work. This is because the TC kernel infrastructure does not allow these to be passed to the kernel. >> >> //Eelco >> >> >> On 4 Jul 2022, at 17:25, Abhiram RN wrote: >> >> Hi Eelco, >> >> I do remember testing with this. I don't have full logs as such. But >> from one of the mail I had sent out I found below logs (this is with the >> change of course) where it's offloaded to TC (PSB). [ Because there seems to >> be support needed in HW driver (which is not present) and hence flows are >> not offloaded to HW which is expected]. But as you can see flows are in >> *dp:tc*.. (I can see 39 packets... so it's not only the first one). Not sure >> if anything got changed post that which might have caused this to fail now. >> >> [root@rhos-nfv-09 ~]# ovs-appctl dpctl/dump-flows -m >> ufid:edb7d581-9f14-42df-9e50-69a4923e1dbd, >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(enp4s0f0_0),packet_type(ns=0/0,id=0/0),eth(src=6a:62:68:b2:61:f6,dst=66:f0:e4:1e:2c:bb),eth_type(0x0800),ipv4(src=0.0.0.0/0.0.0.0,dst=5.5.5.2/255.255.255.254,proto=0/0,tos=0/0x3,ttl=0/0,frag=no >> >> <http://0.0.0.0/0.0.0.0,dst=5.5.5.2/255.255.255.254,proto=0/0,tos=0/0x3,ttl=0/0,frag=no> >> ), packets:39, bytes:3276, used:0.260s, *dp:tc*, >> actions:set(tunnel(tun_id=0x0,src=10.10.10.1,dst=10.10.10.2,ttl=64,flags(key))),erspan_sys,enp4s0f0_1 >> ufid:519d8cfd-9821-467f-9f61-e2bf61cdde60, >> skb_priority(0/0),skb_mark(0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),recirc_id(0),dp_hash(0/0),in_port(enp4s0f0_1),packet_type(ns=0/0,id=0/0),eth(src=66:f0:e4:1e:2c:bb,dst=6a:62:68:b2:61:f6),eth_type(0x0800),ipv4(src=0.0.0.0/0.0.0.0,dst=5.5.5.2/255.255.255.254,proto=0/0,tos=0/0x3,ttl=0/0,frag=no >> >> <http://0.0.0.0/0.0.0.0,dst=5.5.5.2/255.255.255.254,proto=0/0,tos=0/0x3,ttl=0/0,frag=no> >> ), packets:39, bytes:3276, used:0.260s, *dp:tc*, >> actions:enp4s0f0_0,set(tunnel(tun_id=0x0,src=10.10.10.1,dst=10.10.10.2,ttl=64,flags(key))),erspan_sys >> [root@rhos-nfv-09 ~]# >> >> JFYI, >> Without this change still flows won't get offloaded to TC. Below is the >> error what I used to get and the flows used to be in *dp:ovs* >> 2022-02-25T14:33:04.134Z|00151|netdev_offload_tc|INFO|init: failed to >> get ifindex for erspan0: Operation not supported >> 2022-02-25T14:33:04.134Z|00152|netdev_offload|INFO|erspan0: No suitable >> flow API found. >> >> Thanks & Regards, >> Abhiram R N >> >> On Mon, Jul 4, 2022 at 7:48 PM Eelco Chaudron <[email protected] >> <mailto:[email protected]>> wrote: >> >> >> >> On 5 Apr 2022, at 0:48, Ilya Maximets wrote: >> >> > On 3/23/22 14:01, Eelco Chaudron wrote: >> >> >> >> >> >> On 23 Mar 2022, at 11:43, Abhiram R N wrote: >> >> >> >>> When enabling offload for ERSPAN we are seeing one error as below >> >>> >> >>> netdev_offload_tc|INFO|init: failed to get ifindex for erspan0: >> >>> Operation not supported >> >>> netdev_offload|INFO|erspan0: No suitable flow API found. >> >>> >> >>> Adding the NETDEV_VPORT_GET_IFINDEX to ERSPAN device resolves >> this >> >>> error >> >> >> >> The change looks good to me, but there is no real unit test to >> verify this, and I have no real setup to test this. >> >> >> >> So based on a pure visible and compile only check: >> >> >> >> Acked-by: Eelco Chaudron <[email protected] >> <mailto:[email protected]>> >> >> >> >>> Signed-off-by: Abhiram R N <[email protected] >> <mailto:[email protected]>> >> >>> --- >> > >> > I don't see why this can cause any problems or why this callback >> > wasn't set from the beginning, so applied. Thanks! >> > >> > Best regards, Ilya Maximets. >> >> Hi Abhiram/Ilya, >> >> Guess it takes a while but basic ping seems to be failing when >> offloading to TC with t his change. >> >> If you enable TC offload the following tests are failing on latest >> master: >> >> datapath - ping over erspan v1 tunnel FAILED >> (system-traffic.at:467 <http://system-traffic.at:467> ) >> datapath - ping over erspan v2 tunnel FAILED >> (system-traffic.at:504 <http://system-traffic.at:504> ) >> datapath - ping over ip6erspan v1 tunnel FAILED >> (system-traffic.at:544 <http://system-traffic.at:544> ) >> datapath - ping over ip6erspan v2 tunnel FAILED >> (system-traffic.at:585 <http://system-traffic.at:585> ) >> >> >> For example the following diff causes the first test to fail with tc: >> >> >> diff --git a/tests/system-traffic.at <http://system-traffic.at> >> b/tests/system-traffic.at <http://system-traffic.at> >> index e16ea6027..fa6b84b4e 100644 >> --- a/tests/system-traffic.at <http://system-traffic.at> >> +++ b/tests/system-traffic.at <http://system-traffic.at> >> @@ -440,6 +440,9 @@ OVS_CHECK_GRE() >> OVS_CHECK_ERSPAN() >> >> OVS_TRAFFIC_VSWITCHD_START() >> + >> +ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true >> + >> ADD_BR([br-underlay]) >> >> AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) >> >> >> I think it’s because we are not adding any of the erspan fields to >> the offload tunnel encapsulated. >> >> >> Abhiram, wondering if this was really working in your case, as it >> seems only the first packet is passed on. >> >> I guess we might need to undo this change, or you need to come up >> with a fix for this. >> >> >> Cheers, >> >> Eelco >> >> >> >> -- >> Always put yourself in others' shoes. If you feel that it hurts you, it >> probably hurts the other person, too. >> >> Regards >> ABHIRAM >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
