| Message ID | 20230802075118.409395-14-idosch@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | selftests: forwarding: Various fixes | expand |
On Wed, Aug 02, 2023 at 10:51:14AM +0300, Ido Schimmel wrote: > The test installs filters that match on various IP fragments (e.g., no > fragment, first fragment) and expects a certain amount of packets to hit > each filter. This is problematic as the filters are not specific enough > and can match IP packets (e.g., IGMP) generated by the stack, resulting > in failures [1]. [...] > --- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh > +++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh > @@ -104,11 +104,14 @@ tunnel_key_nofrag_test() > local i > > tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \ > - flower ip_flags nofrag action drop > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > + ip_flags nofrag action drop > tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \ > - flower ip_flags firstfrag action drop > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > + ip_flags firstfrag action drop > tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \ > - flower ip_flags nofirstfrag action drop > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > + ip_flags nofirstfrag action drop hello Ido, my 2 cents: is it safe to match on the UDP protocol without changing the mausezahn command line? I see that it's generating generic IP packets at the moment (i.e. it does '-t ip'). Maybe it's more robust to change the test to generate ICMP and then match on the ICMP protocol? thanks!
On Wed, Aug 02, 2023 at 10:30:52AM +0200, Davide Caratti wrote: > On Wed, Aug 02, 2023 at 10:51:14AM +0300, Ido Schimmel wrote: > > The test installs filters that match on various IP fragments (e.g., no > > fragment, first fragment) and expects a certain amount of packets to hit > > each filter. This is problematic as the filters are not specific enough > > and can match IP packets (e.g., IGMP) generated by the stack, resulting > > in failures [1]. > > [...] > > > --- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh > > +++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh > > @@ -104,11 +104,14 @@ tunnel_key_nofrag_test() > > local i > > > > tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \ > > - flower ip_flags nofrag action drop > > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > > + ip_flags nofrag action drop > > tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \ > > - flower ip_flags firstfrag action drop > > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > > + ip_flags firstfrag action drop > > tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \ > > - flower ip_flags nofirstfrag action drop > > + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ > > + ip_flags nofirstfrag action drop > > > hello Ido, my 2 cents: > > is it safe to match on the UDP protocol without changing the mausezahn > command line? I see that it's generating generic IP packets at the > moment (i.e. it does '-t ip'). Maybe it's more robust to change > the test to generate ICMP and then match on the ICMP protocol? My understanding of the test is that it's transmitting IP packets on the VXLAN device and what $swp1 sees are the encapsulated packets (UDP).
On Wed, Aug 2, 2023 at 10:38 AM Ido Schimmel <idosch@idosch.org> wrote: > > On Wed, Aug 02, 2023 at 10:30:52AM +0200, Davide Caratti wrote: [...] > > > > hello Ido, my 2 cents: > > > > is it safe to match on the UDP protocol without changing the mausezahn > > command line? I see that it's generating generic IP packets at the > > moment (i.e. it does '-t ip'). Maybe it's more robust to change > > the test to generate ICMP and then match on the ICMP protocol? > > My understanding of the test is that it's transmitting IP packets on the > VXLAN device and what $swp1 sees are the encapsulated packets (UDP). > Ah, right :) sorry for the noise! Acked-by: Davide Caratti <dcaratti@redhat.com>
diff --git a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh index 5ac184d51809..5a5dd9034819 100755 --- a/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh +++ b/tools/testing/selftests/net/forwarding/tc_tunnel_key.sh @@ -104,11 +104,14 @@ tunnel_key_nofrag_test() local i tc filter add dev $swp1 ingress protocol ip pref 100 handle 100 \ - flower ip_flags nofrag action drop + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ + ip_flags nofrag action drop tc filter add dev $swp1 ingress protocol ip pref 101 handle 101 \ - flower ip_flags firstfrag action drop + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ + ip_flags firstfrag action drop tc filter add dev $swp1 ingress protocol ip pref 102 handle 102 \ - flower ip_flags nofirstfrag action drop + flower src_ip 192.0.2.1 dst_ip 192.0.2.2 ip_proto udp \ + ip_flags nofirstfrag action drop # test 'nofrag' set tc filter add dev h1-et egress protocol all pref 1 handle 1 matchall $tcflags \
The test installs filters that match on various IP fragments (e.g., no fragment, first fragment) and expects a certain amount of packets to hit each filter. This is problematic as the filters are not specific enough and can match IP packets (e.g., IGMP) generated by the stack, resulting in failures [1]. Fix by making the filters more specific and match on more fields in the IP header: Source IP, destination IP and protocol. [1] # timeout set to 0 # selftests: net/forwarding: tc_tunnel_key.sh # TEST: tunnel_key nofrag (skip_hw) [FAIL] # packet smaller than MTU was not tunneled # INFO: Could not test offloaded functionality not ok 89 selftests: net/forwarding: tc_tunnel_key.sh # exit=1 Fixes: 533a89b1940f ("selftests: forwarding: add tunnel_key "nofrag" test case") Reported-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> Closes: https://lore.kernel.org/netdev/adc5e40d-d040-a65e-eb26-edf47dac5b02@alu.unizg.hr/ Signed-off-by: Ido Schimmel <idosch@nvidia.com> Reviewed-by: Petr Machata <petrm@nvidia.com> Tested-by: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr> --- Cc: dcaratti@redhat.com --- tools/testing/selftests/net/forwarding/tc_tunnel_key.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)