| Message ID | CAPWQB7Gn=22jXh3S6iTvN9kEczK6-KX60jzB4zqQB5Zp-GLn-w@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Joe, Thanks for further review. I'll add the changes you have below to the series. I'll take a look at the "check-system-userspace" failure. The "802.1ad - push/pop outer tag" test fails on at least one of my setups. On Thu, Aug 04, 2016 at 05:53:59PM -0700, Joe Stringer wrote: > Thanks for updating the series. > > With the incremental patch below this is looking pretty reliable for > check-kmod/check-kernel on the platforms I can test on, although > there's still some issue with "make check-system-userspace". It seems > like the userspace datapath cannot receive double-tagged packets from > AF_PACKET properly; it's unclear where the issue is yet. > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index ff67be997370..694eeb5f4665 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -85,6 +85,8 @@ ADD_SVLAN(p1, at_ns1, 4094, "10.255.2.2/24") > ADD_CVLAN(p0.4094, at_ns0, 100, "10.2.2.1/24") > ADD_CVLAN(p1.4094, at_ns1, 100, "10.2.2.2/24") > > +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.2.2.2]) > + > NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2 | > FORMAT_PING], [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > @@ -176,7 +178,7 @@ ADD_CVLAN(p1.4094, at_ns1, 100, "fc00:1::2/96") > dnl Linux seems to take a little time to get its IPv6 stack in order. Without > dnl waiting, we get occasional failures due to the following error: > dnl "connect: Cannot assign requested address" > -OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2]) > +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00:1::2]) > > NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:1::2 | > FORMAT_PING], [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > > > > On 2 August 2016 at 08:20, Eric Garver <e@erig.me> wrote: > > This series adds 6 test cases to the "check-kernel" make target for > > 802.1ad. It is meant as a counterpart to the 802.1ad work currently > > going on and being discussed on the dev list. > > > > User space support for 802.1ad is being worked on by Xiao Liang (based > > on Thomas F Herbert's work). > > > > Kernel support is being worked on by myself (also based on Tom's work). > > I will post (and CC ovs-dev) the kernel series once net-next opens again > > for new content. If there is interest I can post that series to ovs-dev > > for discussion in the mean time. > > > > These patches have been tested with Xiao's most recent series and my yet > > to be posted kernel series. > > > > Update v2: > > - Properly skip tests on older versions of OVS and kernel > > - Set CVLAN mtu to 1496 to allow tests to pass on older kernels > > > > Eric Garver (5): > > check-kernel: Add macros to check for and test 802.1ad. > > check-kernel: 802.1ad: Add datapath ping tests for CVLANs. > > check-kernel: 802.1ad: Add conntrack ping tests for CVLANs. > > check-kernel: 802.1ad: Add push/pop test case. > > check-kernel: 802.1ad: Add dot1q-tunnel test case. > > > > tests/system-common-macros.at | 30 ++++- > > tests/system-traffic.at | 268 ++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 297 insertions(+), 1 deletion(-) > > > > -- > > 2.5.5 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
Hi, See two replies below. Thanks again! On Fri, Aug 05, 2016 at 10:03:24AM -0400, Eric Garver wrote: > Joe, > > Thanks for further review. I'll add the changes you have below to the > series. > > I'll take a look at the "check-system-userspace" failure. The "802.1ad > - push/pop outer tag" test fails on at least one of my setups. I fixed the remaining test failures. There is some work to feature flag 802.1ad support to preserve backwards compatibility. I'll hold off on sending out another revision until I can make the necessary test changes to support the feature flag. The feature flag probably also means additional tests to cover both operating modes. > On Thu, Aug 04, 2016 at 05:53:59PM -0700, Joe Stringer wrote: > > Thanks for updating the series. > > > > With the incremental patch below this is looking pretty reliable for > > check-kmod/check-kernel on the platforms I can test on, although > > there's still some issue with "make check-system-userspace". It seems > > like the userspace datapath cannot receive double-tagged packets from > > AF_PACKET properly; it's unclear where the issue is yet. The frames were being sent with the wrong TPID, 0x8100 instead of 0x88a8. So the kernel's VLAN device with proto 802.1ad was dropping them. With the below kernel commit, it works as expected. a0cdfcf39362 ("packet: deliver VLAN TPID to userspace") I believe this commit went into the 3.14 kernel release. This can probably be fixed in OVS userspace dataplane by detecting the double tag by peaking at the ethertype. If the ethertype is 0x8100 and the packet auxdata says there is another tag, then we know it's double tagged and can push the 802.1ad tag. Currently this code just assumes 0x8100 if the kernel didn't pass the TPID. This is all within netdev_linux_rxq_recv_sock(). > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index ff67be997370..694eeb5f4665 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -85,6 +85,8 @@ ADD_SVLAN(p1, at_ns1, 4094, "10.255.2.2/24") > > ADD_CVLAN(p0.4094, at_ns0, 100, "10.2.2.1/24") > > ADD_CVLAN(p1.4094, at_ns1, 100, "10.2.2.2/24") > > > > +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.2.2.2]) > > + > > NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2 | > > FORMAT_PING], [0], [dnl > > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > > ]) > > @@ -176,7 +178,7 @@ ADD_CVLAN(p1.4094, at_ns1, 100, "fc00:1::2/96") > > dnl Linux seems to take a little time to get its IPv6 stack in order. Without > > dnl waiting, we get occasional failures due to the following error: > > dnl "connect: Cannot assign requested address" > > -OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2]) > > +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00:1::2]) > > > > NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:1::2 | > > FORMAT_PING], [0], [dnl > > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > > > > > > > > On 2 August 2016 at 08:20, Eric Garver <e@erig.me> wrote: > > > This series adds 6 test cases to the "check-kernel" make target for > > > 802.1ad. It is meant as a counterpart to the 802.1ad work currently > > > going on and being discussed on the dev list. > > > > > > User space support for 802.1ad is being worked on by Xiao Liang (based > > > on Thomas F Herbert's work). > > > > > > Kernel support is being worked on by myself (also based on Tom's work). > > > I will post (and CC ovs-dev) the kernel series once net-next opens again > > > for new content. If there is interest I can post that series to ovs-dev > > > for discussion in the mean time. > > > > > > These patches have been tested with Xiao's most recent series and my yet > > > to be posted kernel series. > > > > > > Update v2: > > > - Properly skip tests on older versions of OVS and kernel > > > - Set CVLAN mtu to 1496 to allow tests to pass on older kernels > > > > > > Eric Garver (5): > > > check-kernel: Add macros to check for and test 802.1ad. > > > check-kernel: 802.1ad: Add datapath ping tests for CVLANs. > > > check-kernel: 802.1ad: Add conntrack ping tests for CVLANs. > > > check-kernel: 802.1ad: Add push/pop test case. > > > check-kernel: 802.1ad: Add dot1q-tunnel test case. > > > > > > tests/system-common-macros.at | 30 ++++- > > > tests/system-traffic.at | 268 ++++++++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 297 insertions(+), 1 deletion(-) > > > > > > -- > > > 2.5.5 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On 10 August 2016 at 14:55, Eric Garver <e@erig.me> wrote: > Hi, > > See two replies below. > > Thanks again! > > On Fri, Aug 05, 2016 at 10:03:24AM -0400, Eric Garver wrote: >> Joe, >> >> Thanks for further review. I'll add the changes you have below to the >> series. >> >> I'll take a look at the "check-system-userspace" failure. The "802.1ad >> - push/pop outer tag" test fails on at least one of my setups. > > I fixed the remaining test failures. > There is some work to feature flag 802.1ad support to preserve backwards > compatibility. I'll hold off on sending out another revision until I can > make the necessary test changes to support the feature flag. The feature > flag probably also means additional tests to cover both operating modes. OK, fair enough. Whenever you're ready, CC me and I can provide feedback or apply these as appropriate. >> On Thu, Aug 04, 2016 at 05:53:59PM -0700, Joe Stringer wrote: >> > Thanks for updating the series. >> > >> > With the incremental patch below this is looking pretty reliable for >> > check-kmod/check-kernel on the platforms I can test on, although >> > there's still some issue with "make check-system-userspace". It seems >> > like the userspace datapath cannot receive double-tagged packets from >> > AF_PACKET properly; it's unclear where the issue is yet. > > The frames were being sent with the wrong TPID, 0x8100 instead of > 0x88a8. So the kernel's VLAN device with proto 802.1ad was dropping > them. > With the below kernel commit, it works as expected. > > a0cdfcf39362 ("packet: deliver VLAN TPID to userspace") > > I believe this commit went into the 3.14 kernel release. OK, this would affect Ubuntu14.04 (kernel 3.13) for all of the cvlan tests and I see that those tests fail on that kernel. I also see the failure below in the IPv6 case, with >MTU size packets on more recent kernels, do you see this also? 2016-08-04T23:39:12.846Z|00029|netdev_linux|WARN|error sending Ethernet packet on ovs-p1: Message too long Daniele and I took a bit of a look at this, and the simplest path forward is just to drop the MTU on the CVLAN interfaces to 1492. For some reason, some part of the netdev transmit path for AF_PACKET socket doesn't understand the cvlan and is appears to be treating both VLAN tags + the payload as the ethernet payload for MTU calculation, then because 1508 > 1500, returning -EMSGSIZE. For the OVS testsuite, we should probably adjust the MTU like this. If you'd like to follow up on why this is and whether a better fix can be made, that would be great too and a better long-term solution. > This can probably be fixed in OVS userspace dataplane by detecting the > double tag by peaking at the ethertype. If the ethertype is 0x8100 and > the packet auxdata says there is another tag, then we know it's double > tagged and can push the 802.1ad tag. Currently this code just assumes > 0x8100 if the kernel didn't pass the TPID. This is all within > netdev_linux_rxq_recv_sock(). Thanks for the detailed investigation!
On Wed, Aug 10, 2016 at 04:54:08PM -0700, Joe Stringer wrote: > On 10 August 2016 at 14:55, Eric Garver <e@erig.me> wrote: > > Hi, > > > > See two replies below. > > > > Thanks again! > > > > On Fri, Aug 05, 2016 at 10:03:24AM -0400, Eric Garver wrote: > >> Joe, > >> > >> Thanks for further review. I'll add the changes you have below to the > >> series. > >> > >> I'll take a look at the "check-system-userspace" failure. The "802.1ad > >> - push/pop outer tag" test fails on at least one of my setups. > > > > I fixed the remaining test failures. > > There is some work to feature flag 802.1ad support to preserve backwards > > compatibility. I'll hold off on sending out another revision until I can > > make the necessary test changes to support the feature flag. The feature > > flag probably also means additional tests to cover both operating modes. > > OK, fair enough. Whenever you're ready, CC me and I can provide > feedback or apply these as appropriate. > > >> On Thu, Aug 04, 2016 at 05:53:59PM -0700, Joe Stringer wrote: > >> > Thanks for updating the series. > >> > > >> > With the incremental patch below this is looking pretty reliable for > >> > check-kmod/check-kernel on the platforms I can test on, although > >> > there's still some issue with "make check-system-userspace". It seems > >> > like the userspace datapath cannot receive double-tagged packets from > >> > AF_PACKET properly; it's unclear where the issue is yet. > > > > The frames were being sent with the wrong TPID, 0x8100 instead of > > 0x88a8. So the kernel's VLAN device with proto 802.1ad was dropping > > them. > > With the below kernel commit, it works as expected. > > > > a0cdfcf39362 ("packet: deliver VLAN TPID to userspace") > > > > I believe this commit went into the 3.14 kernel release. > > OK, this would affect Ubuntu14.04 (kernel 3.13) for all of the cvlan > tests and I see that those tests fail on that kernel. > > I also see the failure below in the IPv6 case, with >MTU size packets > on more recent kernels, do you see this also? Yes. I've addressed it. See next comment. > 2016-08-04T23:39:12.846Z|00029|netdev_linux|WARN|error sending > Ethernet packet on ovs-p1: Message too long > > Daniele and I took a bit of a look at this, and the simplest path > forward is just to drop the MTU on the CVLAN interfaces to 1492. For > some reason, some part of the netdev transmit path for AF_PACKET > socket doesn't understand the cvlan and is appears to be treating both > VLAN tags + the payload as the ethernet payload for MTU calculation, > then because 1508 > 1500, returning -EMSGSIZE. For the OVS testsuite, > we should probably adjust the MTU like this. If you'd like to follow > up on why this is and whether a better fix can be made, that would be > great too and a better long-term solution. I forgot to mention it, but dropping the MTU to 1492 is exactly what I did. There were also a couple spots in the 802.1ad tests that were using ADD_VLAN(). I replaced those with ADD_CVLAN() to make sure the MTU is set. # ADD_CVLAN([port], [namespace], [vlan-id], [ip-addr]) # # Similar to ADD_VLAN(), but sets MTU. Lower MTU here instead of increase MTU # on bridge/SVLAN because older kernels didn't work. # m4_define([ADD_CVLAN], [ ADD_VLAN([$1], [$2], [$3], [$4]) NS_CHECK_EXEC([$2], [ip link set $1.$3 mtu 1492]) ] ) > > This can probably be fixed in OVS userspace dataplane by detecting the > > double tag by peaking at the ethertype. If the ethertype is 0x8100 and > > the packet auxdata says there is another tag, then we know it's double > > tagged and can push the 802.1ad tag. Currently this code just assumes > > 0x8100 if the kernel didn't pass the TPID. This is all within > > netdev_linux_rxq_recv_sock(). > > Thanks for the detailed investigation! Another note, I believe this means simple port to port forwarding of 802.1ad frames is broken in current OVS userspace dataplane (on < 3.14 kernels). It mistakenly changes the outer TPID. I'll have a go at fixing it independent of the full 802.1ad effort.
diff --git a/tests/system-traffic.at b/tests/system-traffic.at index ff67be997370..694eeb5f4665 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -85,6 +85,8 @@ ADD_SVLAN(p1, at_ns1, 4094, "10.255.2.2/24") ADD_CVLAN(p0.4094, at_ns0, 100, "10.2.2.1/24") ADD_CVLAN(p1.4094, at_ns1, 100, "10.2.2.2/24") +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.2.2.2]) + NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.2.2.2 | FORMAT_PING], [0], [dnl 3 packets transmitted, 3 received, 0% packet loss, time 0ms