On Thu, Mar 2, 2023 at 1:12 AM Simon Horman <[email protected]> wrote: > > On Thu, Mar 02, 2023 at 08:51:18AM +0100, Simon Horman wrote: > > On Wed, Mar 01, 2023 at 02:27:29PM -0800, Han Zhou wrote: > > > On Wed, Mar 1, 2023 at 1:50 AM Simon Horman <[email protected] > > > > wrote: > > > > > > > > On Wed, Feb 22, 2023 at 10:35:25PM -0800, Han Zhou wrote: > > > > > > > > Please add a patch description here. > > > > > > Thank Simon for reviewing. For this commit, I think the commit title tells > > > everything I wanted to describe, so I omitted it here rather than repeating > > > the title. > > > > IMHO something should go here, even if it's just repeating the subject.
Just curious, why would it help to repeat the subject? If the subject is not clear enough I can definitely add more details in the commit message, but I hope we don't make this a rule without good reason. In fact we have many examples in the commit log with just a subject line, in both OVN and OVS repo. > > > > > > > Signed-off-by: Han Zhou <[email protected]> > > > > > --- > > > > > tests/atlocal.in | 3 + > > > > > tests/system-ovn.at | 146 ++++++++++++++++++++++++++++++++++++++++++++ > > > > > 2 files changed, 149 insertions(+) > > > > > > > > ... > > > > > > > > > +NS_EXEC([ns_ls1p1], [arping -U -c 1 -w 2 -I ls1p1 -s 10.0.0.88 > > > 10.0.0.88]) > > > > > > > > I ran into a problem with this when exercising the tests on Ubuntu 22.10. > > > > > > > > When the arping package is installed then -s expects a MAC address > > > > whereas -S expects an ip/hostname. This causes the tests to fail. > > > > > > > > By changing -s to -S here, and for the other invocation of arping, below, > > > I > > > > was able to run the tests successfully for both check-kernel and > > > > check-system-userspace :) > > > > > > > > When, instead, arping is supplied by the iputils-arping package, > > > > then the new tests work unmodified. > > > > > > > > I am not sure what, if anything, we wish to do about such compatibility > > > > issues. But, FWIIW, I believe noticed a similar problem involving nc not > > > so > > > > long ago, although I do not recall specifically in which environment or > > > > which alternate packages. > > > > > > > Thanks a lot for testing this. I didn't test this in Ubuntu, but it's > > > strange that even from this man page of Ubuntu 22.10, -s is still the right > > > one: > > > https://manpages.ubuntu.com/manpages/kinetic/man8/arping.8.html > > I think that is the manpage for the 'iputils-arping' version of the tool. > And the manpage for the 'arping' version can be found here: > > * https://github.com/ThomasHabets/arping/blob/arping-2.x/doc/arping.8 > > > > > > > Regardless, I also see this: > > > > > > If this option is absent, source address is: > > > > > > ... • In Unsolicited ARP mode (with options *-U *or *-A*) set to* destination*. > > > > > > So, hopefully with -U already in the command, we can omit the -s (or -S). > > > Do you mind testing the same by removing the -s option and see if it works > > > in your environment? > > > > Yes, can do. > > Hopefully later today. > > I confirmed that the test is successful for both the 'iputils-arping' and > 'arping' versions of the arping util if the -s option is removed. > > I tested both check-kernel and check-system-userspace. > > # TESTSUITEFLAGS="-k 'virtual port with floating IP'" make check-kernel check-system-userspace > Thanks a lot for testing it! I will remove the -s in v2. Regards, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
