Thu, Jan 10, 2019 at 12:37:15PM CET, Paul.D.Smith(a)metaswitch.com wrote:
Jiri,
Just found some time to look at this again and the bug has existed since release:
042fb5c8818510f18974d63c569699d76d66f64c
rewrite python wrapper in more object nice style ...
Jiri Pirko
Jiri Pirko committed on Nov 21, 2011
In this release you rewrote the file along nice Python class lines and made Team() a
subclass of TeamNetDevice() but you failed to pass the teamdev through to the
TeamNetDevice superclass __init__() method so this code only works if teamdev is zero (the
default).
I don't know how nobody else has hit this (I would hope we're not the first people
to actually use this in anger with multiple 'teamdev's!) but it just doesn't
work.
There is no way that I will have the time to set up tooling to create patches in the way
that you asked for earlier but I have told you the bug, where it is and the fix. FWIW the
fix has been running successfully in our code since about October 2018 and we have quite
complex networking requirements so we're very confident that it is the correct fix.
Hopefully you will find time to look at this and fix it because it seems silly that users
will have to keep applying their own patches for every release if they actually want to
use this code.
Sorry I can't be more helpful but 'time is money' of course and this is all
the help I can manage to squeeze in.
Are you kidding? Instead of writing lenthty emails, you could fix the
patch and resend. I really don't understand, sorry.
+ I just found that your patch is wrong. You pass "teamdev" which is
ifindex to super - TeamNetDevice constructor which accepts ifindex -
integer.
When you call "Team" constructor, you should not pass ifindex but
interface name. Otherwise you endup in else branch of:
ifindex = self._conv.get_ifindex(teamdev) if teamdev else 0
So basically your patch tries to workaround your own bug.
>
>Regards,
>Paul D Smith.
>
>
>-----Original Message-----
>From: Jiri Pirko <jiri(a)resnulli.us>
>Sent: 14 August 2018 09:49
>To: Paul D. Smith <Paul.D.Smith(a)metaswitch.com>
>Subject: Re: [patch libteam] Pass device ID through to super class init.
>
>Tue, Aug 14, 2018 at 10:29:24AM CEST, Paul.D.Smith(a)metaswitch.com wrote:
>>Jiri,
>>
>>I'm afraid I can't install git-email because this requires an update to my
core Git install and my employers like everyone to use the same level of git. However
here are some notes on the fix that I sent earlier.
>>
>>Without the patch, we were hitting this bug:
>>
>> APPLICATION BUG: route/link.c:1233:rtnl_link_build_get_request: ifindex or name
must be specified
>> python: route/link.c:1233: rtnl_link_build_get_request: Assertion `0'
failed.
>> Aborted (core dumped)
>>
>>Once we pass through the correct ifindex as a parameter, all works fine.
>
>I have no doubts that the patch helps
>
>
>>
>>Looking at history via Github, the last change to the code we patched was over 6
years ago in commit 'a7702de3c6b2dfb0e2fc951ab4c6dbe0f5d1a983' and that change
looks in no way related to this problem.
>
>So it has to be another patch. Please find it.
>
>>
>>We do have some concern as to how this bug might have been missed for so long and
wonder whether we might be somehow misusing the interface but at present we can't see
how - it looks like this fix is required and always has been.
>>
>>I hope this gives you enough to go on - sorry we can't bundle this all up in
your preferred format.
>
>
>Please repost the patch with the proper description (now it is empty).
>Please fill up "Fixes:"
>If you can do "git send email", at least mimic it (No headers in the email
body)
>
>>
>>Regards,
>>Paul D.Smith.
>>
>>-----Original Message-----
>>From: Jiri Pirko <jiri(a)resnulli.us>
>>Sent: 08 August 2018 10:07
>>To: Paul D. Smith <Paul.D.Smith(a)metaswitch.com>
>>Cc: libteam(a)lists.fedorahosted.org
>>Subject: Re: [patch libteam] Pass device ID through to super class init.
>>
>>Wed, Aug 08, 2018 at 11:04:13AM CEST, Paul.D.Smith(a)metaswitch.com wrote:
>>>Jiri,
>>>
>>>I following the instructions as far as, and including, using the 'git
format-patch' command but my version of Git does not have the 'git send-email'
command so I just cut-n-pasted the patch into an e-mail by hand.
>>
>>Okay. That's the problem. "git send-email" is usually distributed in
a separate package. In Fedora it is in "git-email" package.
>>
>>
>>>
>>>The motivation for the change is that we found that the code just didn't
work; my understanding is that regardless of which team device we were attempting to
program, the superclass init() always attempted to program the '0' team device.
>>>
>>>I'll drop you more info next week when my colleague, who hit this when
developing our code and can probably give you a more detailed answer, returns from
holiday.
>>
>>Okay. So it is a bugfix. Yeah, please provide this info in the patch description.
Also, if you find the offending commit tha actually caused this issue, it is fine to add
line similar to this:
>>Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool")
>>
>>Please see "git log" and find for "Fixes" to see how is it
used.
>>
>>Thanks!
>>
>>
>>>
>>>Regards,
>>>Paul DS.
>>>
>>>
>>>-----Original Message-----
>>>From: Jiri Pirko <jiri(a)resnulli.us>
>>>Sent: 08 August 2018 09:39
>>>To: Paul D. Smith <Paul.D.Smith(a)metaswitch.com>
>>>Cc: libteam(a)lists.fedorahosted.org
>>>Subject: Re: [patch libteam] Pass device ID through to super class init.
>>>
>>>Mon, Aug 06, 2018 at 02:46:30PM CEST, Paul.D.Smith(a)metaswitch.com wrote:
>>>>From 34bccf0221af3f063686d58704d1255eab5f15b9 Mon Sep 17 00:00:00
>>>>2001
>>>>From: "Paul D.Smith" <paul.d.smith(a)metaswitch.com>
>>>>Date: Mon, 6 Aug 2018 13:41:28 +0100
>>>>Subject: [patch libteam] Pass device ID through to super class init.
>>>
>>>These 4 lines does not supposed to be there. Odd. Did you use "git
format-patch" and "git send-email"?
>>>
>>>Also, please provide a bit of description of the change. At least some
motivation for the change.
>>>
>>>Thanks!
>>>
>>>
>>>>
>>>>Signed-off-by: Paul D.Smith <paul.d.smith(a)metaswitch.com>
>>>>---
>>>>binding/python/team/core.py | 2 +-
>>>>1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>>diff --git a/binding/python/team/core.py
>>>>b/binding/python/team/core.py index 54161bf..e784930 100644
>>>>--- a/binding/python/team/core.py
>>>>+++ b/binding/python/team/core.py
>>>>@@ -348,7 +348,7 @@ class Team(TeamNetDevice):
>>>> if not th:
>>>> raise TeamLibError("Failed to allocate team
handle.")
>>>>
>>>>- super(Team, self).__init__(th)
>>>>+ super(Team, self).__init__(th, teamdev)
>>>>
>>>> if isinstance(teamdev, str):
>>>> err = 0
>>>>--
>>>>2.9.2