From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xin Long Subject: Re: [PATCHv2 net-next 2/2] selftests: add a selftest for directed broadcast forwarding Date: Thu, 5 Jul 2018 01:56:23 +0800 Message-ID: References: <62ecbcf0c905dde3bfde51cd260e2f7c59e21028.1530512974.git.lucien.xin@gmail.com> <03b43b2dbda208510514082b2bd94643c3a6580c.1530512974.git.lucien.xin@gmail.com> <5e5f7edf-1313-cfef-9005-b05ec9051b25@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: network dev , davem , Davide Caratti , Ido Schimmel To: David Ahern Return-path: Received: from mail-io0-f193.google.com ([209.85.223.193]:36350 "EHLO mail-io0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752019AbeGDR4Y (ORCPT ); Wed, 4 Jul 2018 13:56:24 -0400 Received: by mail-io0-f193.google.com with SMTP id k3-v6so5511776iog.3 for ; Wed, 04 Jul 2018 10:56:24 -0700 (PDT) In-Reply-To: <5e5f7edf-1313-cfef-9005-b05ec9051b25@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On Wed, Jul 4, 2018 at 3:23 AM, David Ahern wrote: > On 7/3/18 5:36 AM, Xin Long wrote: >> On Mon, Jul 2, 2018 at 11:12 PM, David Ahern wrote: >>> On 7/2/18 12:30 AM, Xin Long wrote: >>>> +ping_ipv4() >>>> +{ >>>> + sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0 >>>> + bc_forwarding_disable >>>> + ping_test $h1 198.51.100.255 >>>> + >>>> + iptables -A INPUT -i vrf-r1 -p icmp -j DROP >>>> + bc_forwarding_restore >>>> + bc_forwarding_enable >>>> + ping_test $h1 198.51.100.255 >>>> + >>>> + bc_forwarding_restore >>>> + iptables -D INPUT -i vrf-r1 -p icmp -j DROP >>>> + sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts >>>> +} >>> >>> Both tests fail for me: >>> TEST: ping [FAIL] >>> TEST: ping [FAIL] >> I think 'ip vrf exec ...' is not working in your env, while >> the testing is using "ip vrf exec vrf-h1 ping ..." >> >> You can test it by: >> # ip link add dev vrf-test type vrf table 1111 >> # ip vrf exec vrf-test ls > > well, that's embarrassing. yes, I updated ip and forgot to apply the bpf > workaround to define the syscall number (not defined in jessie). > >> >>> >>> Why the need for the iptables rule? >> This iptables rule is to block the echo_request packet going to >> route's local_in. >> When bc_forwarding is NOT doing forwarding well but the packet >> goes to the route's local_in, it will fail. >> >> Without this rule, the 2nd ping will always succeed, we can't tell the >> echo_reply is from route or h2. >> >> Or you have a better way to test this? > > your commands are not a proper test. The test should succeed and fail > based on the routing lookup, not iptables rules. A proper test can be done easily with netns, as vrf can't isolate much. I don't want to bother forwarding/ directory with netns, so I will probably just drop this selftest, and let the feature patch go first. What do you think? > >> >>> >>> And, PAUSE_ON_FAIL is not working to take a look at why tests are >>> failing. e.g., >>> >>> PAUSE_ON_FAIL=yes ./router_broadcast.sh >>> >>> just continues on. Might be something with the infrastructure scripts. >> Yes, in ./router_broadcast.sh, it loads lib.sh where it loads forwarding.config >> where it has "PAUSE_ON_FAIL=no", which would override your >> "PAUSE_ON_FAIL=yes". >> > > ack. bit by that as well.