Re: [libvirt] [tck PATCH 5/4] nwfilter: account for more leading 0's in MAC addresses in ebtables output

On Mon, Feb 11, 2019 at 09:51:09AM -0500, Laine Stump wrote: > On 2/11/19 6:11 AM, Daniel P. Berrangé wrote: >> On Mon, Feb 11, 2019 at 06:07:40AM -0500, Laine Stump wrote: >>> On Mon, Feb 11, 2019, 5:50 AM Daniel P. Berrangé <berrange(a)redhat.com&gt; >>> wrote: >>> >>>> On Sat, Feb 09, 2019 at 02:03:05PM -0500, Laine Stump wrote: >>>>> Since this test (050-apply-verify-host.t), we can't use a regexp in >>>>> the string to be compared. The fix method that leads to the least >>>>> changes is to use sed to remove potential leading 0's. >>>>> >>>>> Signed-off-by: Laine Stump <laine(a)laine.org&gt; >>>>> --- >>>>> >>>>> (These changes fix *almost* all failures in >>>>> nwfilter/050-apply-verify-host.t on RHEL8. The rest look like they >>>>> might be legitimate problems with ebtables and IPv6) >>>> Interesting, I swear I have previously got that test to succeed so >>>> wonder what's changed since then ! >>>> >>> I figured it out yesterday evening but haven't gotten a chance to post it >>> yet. I was being alarmist - its not a behavioral difference, but just a >>> difference in how ipv6 addresses are formatted. The original ebtables >>> reports ipv6 addresses with a netmask (/ffff:ffff:ffff:ffff:8000::) while >>> the iptables-ebtables package that RHRL8 is now using reports it with a >>> prefix (/65). They probably hadn't switched packages yet the last time you >>> ran the test. I have a patch that modifies the expected output (and uses >>> sed to modify the output from 'older' hosts, similar to what you had done >>> for RARP vs 0x8035) and will post it in a few hours, once I've had coffee >>> and tested on both types of host. >> IMHO that should be reported as a bug against ebtables. The output format >> of the new tools should be 100% identical tothe old tools. Changing from >> a netmask to a prefix is a significant semantic difference that will break >> too many uses. > > I thought about that, but wasn't feeling that ambitious since it was Sunday. > If this is considered a bug, then changing the MAC address format from > %x:%x:%x:%x:%x:%x to %0x:%0x:%0x:%0x:%0x:%0x should also be considered a > bug. Yes, I thought about that too. I think it would be worth raising that with the maintainers to validate this was intentional. I get the feeling they'll say that the old behaviour was a clear bug. If anything I'd probably ask them to fix the old impl to not skip leading zeros too. IMHO mac addrs should always be exactly the same length when printed.

> I'll still post a patch to remedy it in the tests, but won't push it (unless > you think that's worthwhile) and will file a bug instead. Yep, lets at least wait & see what response we get to a bug report on this issue.