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

[Laine Stump]

On 2/11/19 9:56 AM, Daniel P. Berrangé wrote:
> 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 at redhat.com>
>>>> 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 at laine.org>
>>>>>> ---
>>>>>>
>>>>>> (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.


Yeah, that was another of the things that caused my ambivalence in 
reporting it as a bug - I also think MAC addresses should be fixed 
length (and I think that using netmasks for IPv6 is an exercise in 
incoherence  - a prefix is much easier to consume).


>
>> 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.


I filed this BZ (and marked it as a regression):

   https://bugzilla.redhat.com/show_bug.cgi?id=1674536

In the meantime, I also patched my local directory to work around the 
difference (as well as adding the patch you sent today that you haven't 
yet pushed) built a RHEL8 rpm, and updated my personal repo:

   https://people.redhat.com/lstump/libvirt-tck-rhel8/