[dpdk-dev,v6,3/4] bond mode 4: allow external state machine
Commit Message
From: Eric Kinzie <ekinzie@brocade.com>
Provide functions to allow an external 802.3ad state machine to transmit
and recieve LACPDUs and to set the collection/distribution flags on
slave interfaces.
Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
functions.
Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
drivers/net/bonding/Makefile | 2 +-
drivers/net/bonding/rte_eth_bond_8023ad.c | 240 ++++++++++++++++++++-
drivers/net/bonding/rte_eth_bond_8023ad.h | 44 ++++
drivers/net/bonding/rte_eth_bond_8023ad_private.h | 28 +++
drivers/net/bonding/rte_eth_bond_version.map | 10 +
5 files changed, 321 insertions(+), 3 deletions(-)
Comments
2015-10-19 08:36, Eric Kinzie:
> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> functions.
[...]
> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
[...]
> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> + bond_mode_8023ad_setup_v22);
I'm sorry it doesn't work well when trying to build a combined lib:
ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
The symbols are OK in the .o file:
0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
I don't understand the problem and I am considering disabling versioning in
combined library.
Any idea?
2015-11-01 23:17, Ferruh Yigit:
> Fixes following error:
> LD libdpdk.so
> /usr/bin/ld: /root/dpdk/build/lib/libdpdk.so: version node not found
> for symbol <function>@DPDK_x.y
> --- a/mk/rte.sharelib.mk
> +++ b/mk/rte.sharelib.mk
> @@ -40,6 +40,7 @@ LIB_ONE := lib$(RTE_LIBNAME).so
> else
> LIB_ONE := lib$(RTE_LIBNAME).a
> endif
> +SHARED_LDFLAGS += --version-script=./lib/libdpdk.map
> endif
It looks to be the right thing to do.
> ifeq ($(LINK_USING_CC),1)
> # Override the definition of LD here, since we're linking with CC
> LD := $(CC) $(CPU_CFLAGS)
> -O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
> +O_TO_S = $(LD) $(call linkerprefix,$(SHARED_LDFLAGS)) \
> -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> else
> -O_TO_S = $(LD) $(CPU_LDFLAGS) \
> +O_TO_S = $(LD) $(SHARED_LDFLAGS) \
> -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> endif
Why do you remove CPU_LDFLAGS?
2015-11-02 00:05, Ferruh Yigit:
> On Mon, Nov 02, 2015 at 12:24:51AM +0100, Thomas Monjalon wrote:
> > > ifeq ($(LINK_USING_CC),1)
> > > # Override the definition of LD here, since we're linking with CC
> > > LD := $(CC) $(CPU_CFLAGS)
> > > -O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
> > > +O_TO_S = $(LD) $(call linkerprefix,$(SHARED_LDFLAGS)) \
> > > -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> > > else
> > > -O_TO_S = $(LD) $(CPU_LDFLAGS) \
> > > +O_TO_S = $(LD) $(SHARED_LDFLAGS) \
> > > -shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> > > endif
> >
> > Why do you remove CPU_LDFLAGS?
>
> CPU_LDFLAGS shared by other libraries and confilicts with this usage.
Sorry I don't understand.
CPU_LDFLAGS is -melf_i386 for Intel 32-bit.
Why removing it for combined library? What is the conflict?
On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> 2015-10-19 08:36, Eric Kinzie:
>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
>> functions.
> [...]
>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> [...]
>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
>> + bond_mode_8023ad_setup_v22);
>
> I'm sorry it doesn't work well when trying to build a combined lib:
>
> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
>
> The symbols are OK in the .o file:
> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
>
> I don't understand the problem and I am considering disabling versioning in
> combined library.
>
> Any idea?
>
The .map additions look incorrect to me:
> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> index 22bd920..7f78717 100644
> --- a/drivers/net/bonding/rte_eth_bond_version.map
> +++ b/drivers/net/bonding/rte_eth_bond_version.map
> @@ -17,6 +17,9 @@ DPDK_2.0 {
> rte_eth_bond_slaves_get;
> rte_eth_bond_xmit_policy_get;
> rte_eth_bond_xmit_policy_set;
> + rte_eth_bond_8023ad_ext_collect;
> + rte_eth_bond_8023ad_ext_distrib;
> + rte_eth_bond_8023ad_ext_slowtx;
These symbols didn't exist in DPDK 2.0 but are only being added here. So
why are they being added to the 2.0 section?
>
> local: *;
> };
> @@ -27,3 +30,10 @@ DPDK_2.1 {
> rte_eth_bond_free;
>
> } DPDK_2.0;
> +
> +DPDK_2.2 {
> + local
> +
> + bond_mode_8023ad_conf_get;
> + bond_mode_8023ad_setup;
> +} DPDK_2.1;
These are marked local, as in, "not exported" which doesn't seem right.
Also they're lacking the rte_eth_ prefix. AFAICS this is what the symbol
export map should look like here:
DPDK_2.2 {
global:
rte_eth_bond_8023ad_ext_collect;
rte_eth_bond_8023ad_ext_distrib;
rte_eth_bond_8023ad_ext_slowtx;
rte_eth_bond_mode_8023ad_conf_get;
rte_eth_bond_mode_8023ad_setup;
} DPDK_2.1;
That said, problems with symbol versioning and the combined library were
predicted from the start, once the individual library versions start
advancing. For one, the combined library itself is not versioned at all
it is offending the ABI policy all the time.
Replacing the library with a linker script, as suggested before by Neil
Horman and Sergio Gonzales Monroy would eliminate these problems.
- Panu -
On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> >2015-10-19 08:36, Eric Kinzie:
> >> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> >> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> >> functions.
> >[...]
> >>+VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> >[...]
> >>+BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>+MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> >>+ bond_mode_8023ad_setup_v22);
> >
> >I'm sorry it doesn't work well when trying to build a combined lib:
> >
> >ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
> >
> >The symbols are OK in the .o file:
> >0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> >0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> >0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> >0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> >0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
> >
> >I don't understand the problem and I am considering disabling versioning in
> >combined library.
> >
> >Any idea?
> >
>
> The .map additions look incorrect to me:
>
> >diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> >index 22bd920..7f78717 100644
> >--- a/drivers/net/bonding/rte_eth_bond_version.map
> >+++ b/drivers/net/bonding/rte_eth_bond_version.map
> >@@ -17,6 +17,9 @@ DPDK_2.0 {
> > rte_eth_bond_slaves_get;
> > rte_eth_bond_xmit_policy_get;
> > rte_eth_bond_xmit_policy_set;
> >+ rte_eth_bond_8023ad_ext_collect;
> >+ rte_eth_bond_8023ad_ext_distrib;
> >+ rte_eth_bond_8023ad_ext_slowtx;
>
> These symbols didn't exist in DPDK 2.0 but are only being added
> here. So why are they being added to the 2.0 section?
Yes, I think these should probably be moved.
> >
> > local: *;
> > };
> >@@ -27,3 +30,10 @@ DPDK_2.1 {
> > rte_eth_bond_free;
> >
> > } DPDK_2.0;
> >+
> >+DPDK_2.2 {
> >+ local
> >+
> >+ bond_mode_8023ad_conf_get;
> >+ bond_mode_8023ad_setup;
> >+} DPDK_2.1;
>
> These are marked local, as in, "not exported" which doesn't seem
> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
> the symbol export map should look like here:
These were not exported to begin with. But after versioning these
functions, they are exported unless explicitly declared to be local here.
> DPDK_2.2 {
> global:
>
> rte_eth_bond_8023ad_ext_collect;
> rte_eth_bond_8023ad_ext_distrib;
> rte_eth_bond_8023ad_ext_slowtx;
> rte_eth_bond_mode_8023ad_conf_get;
> rte_eth_bond_mode_8023ad_setup;
> } DPDK_2.1;
>
> That said, problems with symbol versioning and the combined library
> were predicted from the start, once the individual library versions
> start advancing. For one, the combined library itself is not
> versioned at all it is offending the ABI policy all the time.
>
> Replacing the library with a linker script, as suggested before by
> Neil Horman and Sergio Gonzales Monroy would eliminate these
> problems.
>
> - Panu -
>
>
On 11/02/2015 06:42 PM, Eric Kinzie wrote:
> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
>>> 2015-10-19 08:36, Eric Kinzie:
>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
>>>> functions.
>>> [...]
>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
>>> [...]
>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
>>>> + bond_mode_8023ad_setup_v22);
>>>
>>> I'm sorry it doesn't work well when trying to build a combined lib:
>>>
>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
>>>
>>> The symbols are OK in the .o file:
>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
>>>
>>> I don't understand the problem and I am considering disabling versioning in
>>> combined library.
>>>
>>> Any idea?
>>>
>>
>> The .map additions look incorrect to me:
>>
>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
>>> index 22bd920..7f78717 100644
>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
>>> rte_eth_bond_slaves_get;
>>> rte_eth_bond_xmit_policy_get;
>>> rte_eth_bond_xmit_policy_set;
>>> + rte_eth_bond_8023ad_ext_collect;
>>> + rte_eth_bond_8023ad_ext_distrib;
>>> + rte_eth_bond_8023ad_ext_slowtx;
>>
>> These symbols didn't exist in DPDK 2.0 but are only being added
>> here. So why are they being added to the 2.0 section?
>
> Yes, I think these should probably be moved.
>
>
>>>
>>> local: *;
>>> };
>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
>>> rte_eth_bond_free;
>>>
>>> } DPDK_2.0;
>>> +
>>> +DPDK_2.2 {
>>> + local
>>> +
>>> + bond_mode_8023ad_conf_get;
>>> + bond_mode_8023ad_setup;
>>> +} DPDK_2.1;
>>
>> These are marked local, as in, "not exported" which doesn't seem
>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
>> the symbol export map should look like here:
>
>
> These were not exported to begin with. But after versioning these
> functions, they are exported unless explicitly declared to be local here.
>
And this does not ring any warning bells? :)
Sorry, I was not looking at the patch as a whole. You're declaring these
symbols as exported with the versioning macros, eg
BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
...and then explicitly telling it to not export them by declaring local,
and then we wonder why it has trouble finding the symbols.
The versioning macros wont invent the librte_ prefix for you, you need
to rename the functions accordingly.
But all this versioning gymnastics is moot anyway because you declare
the ABI incompatible:
-LIBABIVER := 1
+LIBABIVER := 2
This changes the library soname, so no binary compiled against the
previous version can possibly use it anymore. As in, by definition there
can be no callers of the _v20 variants after this ABI version bump.
- Panu -
2015-11-03 08:48, Panu Matilainen:
> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
> > On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
> >> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> >>> 2015-10-19 08:36, Eric Kinzie:
> >>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> >>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> >>>> functions.
> >>> [...]
> >>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> >>> [...]
> >>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> >>>> + bond_mode_8023ad_setup_v22);
> >>>
> >>> I'm sorry it doesn't work well when trying to build a combined lib:
> >>>
> >>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
> >>>
> >>> The symbols are OK in the .o file:
> >>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> >>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> >>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> >>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> >>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
> >>>
> >>> I don't understand the problem and I am considering disabling versioning in
> >>> combined library.
> >>>
> >>> Any idea?
> >>>
> >>
> >> The .map additions look incorrect to me:
> >>
> >>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> >>> index 22bd920..7f78717 100644
> >>> --- a/drivers/net/bonding/rte_eth_bond_version.map
> >>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
> >>> @@ -17,6 +17,9 @@ DPDK_2.0 {
> >>> rte_eth_bond_slaves_get;
> >>> rte_eth_bond_xmit_policy_get;
> >>> rte_eth_bond_xmit_policy_set;
> >>> + rte_eth_bond_8023ad_ext_collect;
> >>> + rte_eth_bond_8023ad_ext_distrib;
> >>> + rte_eth_bond_8023ad_ext_slowtx;
> >>
> >> These symbols didn't exist in DPDK 2.0 but are only being added
> >> here. So why are they being added to the 2.0 section?
> >
> > Yes, I think these should probably be moved.
> >
> >
> >>>
> >>> local: *;
> >>> };
> >>> @@ -27,3 +30,10 @@ DPDK_2.1 {
> >>> rte_eth_bond_free;
> >>>
> >>> } DPDK_2.0;
> >>> +
> >>> +DPDK_2.2 {
> >>> + local
> >>> +
> >>> + bond_mode_8023ad_conf_get;
> >>> + bond_mode_8023ad_setup;
> >>> +} DPDK_2.1;
> >>
> >> These are marked local, as in, "not exported" which doesn't seem
> >> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
> >> the symbol export map should look like here:
> >
> >
> > These were not exported to begin with. But after versioning these
> > functions, they are exported unless explicitly declared to be local here.
> >
>
> And this does not ring any warning bells? :)
>
> Sorry, I was not looking at the patch as a whole. You're declaring these
> symbols as exported with the versioning macros, eg
>
> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>
> ...and then explicitly telling it to not export them by declaring local,
> and then we wonder why it has trouble finding the symbols.
> The versioning macros wont invent the librte_ prefix for you, you need
> to rename the functions accordingly.
>
> But all this versioning gymnastics is moot anyway because you declare
> the ABI incompatible:
>
> -LIBABIVER := 1
> +LIBABIVER := 2
>
> This changes the library soname, so no binary compiled against the
> previous version can possibly use it anymore. As in, by definition there
> can be no callers of the _v20 variants after this ABI version bump.
Panu, thanks for your careful review.
We must remove the private functions from the export map.and not bump LIBABIVER.
On 11/03/2015 10:16 AM, Thomas Monjalon wrote:
> 2015-11-03 08:48, Panu Matilainen:
>> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
>>> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
>>>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
>>>>> 2015-10-19 08:36, Eric Kinzie:
>>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
>>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
>>>>>> functions.
>>>>> [...]
>>>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
>>>>> [...]
>>>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
>>>>>> + bond_mode_8023ad_setup_v22);
>>>>>
>>>>> I'm sorry it doesn't work well when trying to build a combined lib:
>>>>>
>>>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
>>>>>
>>>>> The symbols are OK in the .o file:
>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
>>>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
>>>>>
>>>>> I don't understand the problem and I am considering disabling versioning in
>>>>> combined library.
>>>>>
>>>>> Any idea?
>>>>>
>>>>
>>>> The .map additions look incorrect to me:
>>>>
>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
>>>>> index 22bd920..7f78717 100644
>>>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
>>>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
>>>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
>>>>> rte_eth_bond_slaves_get;
>>>>> rte_eth_bond_xmit_policy_get;
>>>>> rte_eth_bond_xmit_policy_set;
>>>>> + rte_eth_bond_8023ad_ext_collect;
>>>>> + rte_eth_bond_8023ad_ext_distrib;
>>>>> + rte_eth_bond_8023ad_ext_slowtx;
>>>>
>>>> These symbols didn't exist in DPDK 2.0 but are only being added
>>>> here. So why are they being added to the 2.0 section?
>>>
>>> Yes, I think these should probably be moved.
>>>
>>>
>>>>>
>>>>> local: *;
>>>>> };
>>>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
>>>>> rte_eth_bond_free;
>>>>>
>>>>> } DPDK_2.0;
>>>>> +
>>>>> +DPDK_2.2 {
>>>>> + local
>>>>> +
>>>>> + bond_mode_8023ad_conf_get;
>>>>> + bond_mode_8023ad_setup;
>>>>> +} DPDK_2.1;
>>>>
>>>> These are marked local, as in, "not exported" which doesn't seem
>>>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
>>>> the symbol export map should look like here:
>>>
>>>
>>> These were not exported to begin with. But after versioning these
>>> functions, they are exported unless explicitly declared to be local here.
>>>
>>
>> And this does not ring any warning bells? :)
>>
>> Sorry, I was not looking at the patch as a whole. You're declaring these
>> symbols as exported with the versioning macros, eg
>>
>> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>
>> ...and then explicitly telling it to not export them by declaring local,
>> and then we wonder why it has trouble finding the symbols.
>> The versioning macros wont invent the librte_ prefix for you, you need
>> to rename the functions accordingly.
>>
>> But all this versioning gymnastics is moot anyway because you declare
>> the ABI incompatible:
>>
>> -LIBABIVER := 1
>> +LIBABIVER := 2
>>
>> This changes the library soname, so no binary compiled against the
>> previous version can possibly use it anymore. As in, by definition there
>> can be no callers of the _v20 variants after this ABI version bump.
>
> Panu, thanks for your careful review.
> We must remove the private functions from the export map.and not bump LIBABIVER.
>
Actually the LIBABIVER bump seems appropriate since a public struct
(rte_eth_bond_8023ad_conf) changes size.
As for the rest, I'm not quite sure what the patch is trying to achieve.
I initially assumed its trying to add a new incompatible version of
rte_bond_mode_8023ad_setup() and rte_eth_bond_8023ad_conf_get() which
could be a perfectly reasonable thing to do in some circumstances. As it
is, the patch actually adds versioning to an internal symbol which just
doesn't make sense at all. Symbol versioning is only meaningful for
public symbols.
- Panu -
2015-11-03 10:30, Panu Matilainen:
> On 11/03/2015 10:16 AM, Thomas Monjalon wrote:
> > 2015-11-03 08:48, Panu Matilainen:
> >> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
> >>> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
> >>>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> >>>>> 2015-10-19 08:36, Eric Kinzie:
> >>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> >>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> >>>>>> functions.
> >>>>> [...]
> >>>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> >>>>> [...]
> >>>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> >>>>>> + bond_mode_8023ad_setup_v22);
> >>>>>
> >>>>> I'm sorry it doesn't work well when trying to build a combined lib:
> >>>>>
> >>>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
> >>>>>
> >>>>> The symbols are OK in the .o file:
> >>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> >>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> >>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> >>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> >>>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
> >>>>>
> >>>>> I don't understand the problem and I am considering disabling versioning in
> >>>>> combined library.
> >>>>>
> >>>>> Any idea?
> >>>>>
> >>>>
> >>>> The .map additions look incorrect to me:
> >>>>
> >>>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>> index 22bd920..7f78717 100644
> >>>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
> >>>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
> >>>>> rte_eth_bond_slaves_get;
> >>>>> rte_eth_bond_xmit_policy_get;
> >>>>> rte_eth_bond_xmit_policy_set;
> >>>>> + rte_eth_bond_8023ad_ext_collect;
> >>>>> + rte_eth_bond_8023ad_ext_distrib;
> >>>>> + rte_eth_bond_8023ad_ext_slowtx;
> >>>>
> >>>> These symbols didn't exist in DPDK 2.0 but are only being added
> >>>> here. So why are they being added to the 2.0 section?
> >>>
> >>> Yes, I think these should probably be moved.
> >>>
> >>>
> >>>>>
> >>>>> local: *;
> >>>>> };
> >>>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
> >>>>> rte_eth_bond_free;
> >>>>>
> >>>>> } DPDK_2.0;
> >>>>> +
> >>>>> +DPDK_2.2 {
> >>>>> + local
> >>>>> +
> >>>>> + bond_mode_8023ad_conf_get;
> >>>>> + bond_mode_8023ad_setup;
> >>>>> +} DPDK_2.1;
> >>>>
> >>>> These are marked local, as in, "not exported" which doesn't seem
> >>>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
> >>>> the symbol export map should look like here:
> >>>
> >>>
> >>> These were not exported to begin with. But after versioning these
> >>> functions, they are exported unless explicitly declared to be local here.
> >>>
> >>
> >> And this does not ring any warning bells? :)
> >>
> >> Sorry, I was not looking at the patch as a whole. You're declaring these
> >> symbols as exported with the versioning macros, eg
> >>
> >> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>
> >> ...and then explicitly telling it to not export them by declaring local,
> >> and then we wonder why it has trouble finding the symbols.
> >> The versioning macros wont invent the librte_ prefix for you, you need
> >> to rename the functions accordingly.
> >>
> >> But all this versioning gymnastics is moot anyway because you declare
> >> the ABI incompatible:
> >>
> >> -LIBABIVER := 1
> >> +LIBABIVER := 2
> >>
> >> This changes the library soname, so no binary compiled against the
> >> previous version can possibly use it anymore. As in, by definition there
> >> can be no callers of the _v20 variants after this ABI version bump.
> >
> > Panu, thanks for your careful review.
> > We must remove the private functions from the export map.and not bump LIBABIVER.
>
> Actually the LIBABIVER bump seems appropriate since a public struct
> (rte_eth_bond_8023ad_conf) changes size.
Not sure. It is added at the end of a struct which is not embedded elsewhere.
> As for the rest, I'm not quite sure what the patch is trying to achieve.
> I initially assumed its trying to add a new incompatible version of
> rte_bond_mode_8023ad_setup() and rte_eth_bond_8023ad_conf_get() which
> could be a perfectly reasonable thing to do in some circumstances. As it
> is, the patch actually adds versioning to an internal symbol which just
> doesn't make sense at all. Symbol versioning is only meaningful for
> public symbols.
Yes
On 11/03/2015 01:02 PM, Ferruh Yigit wrote:
> On Tue, Nov 03, 2015 at 08:48:16AM +0200, Panu Matilainen wrote:
>> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
>>> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
>>>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
>>>>> 2015-10-19 08:36, Eric Kinzie:
>>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
>>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
>>>>>> functions.
>>>>> [...]
>>>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
>>>>> [...]
>>>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
>>>>>> + bond_mode_8023ad_setup_v22);
>>>>>
>>>>> I'm sorry it doesn't work well when trying to build a combined lib:
>>>>>
>>>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
>>>>>
>>>>> The symbols are OK in the .o file:
>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
>>>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
>>>>>
>>>>> I don't understand the problem and I am considering disabling versioning in
>>>>> combined library.
>>>>>
>>>>> Any idea?
>>>>>
>>>>
>>>> The .map additions look incorrect to me:
>>>>
>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
>>>>> index 22bd920..7f78717 100644
>>>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
>>>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
>>>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
>>>>> rte_eth_bond_slaves_get;
>>>>> rte_eth_bond_xmit_policy_get;
>>>>> rte_eth_bond_xmit_policy_set;
>>>>> + rte_eth_bond_8023ad_ext_collect;
>>>>> + rte_eth_bond_8023ad_ext_distrib;
>>>>> + rte_eth_bond_8023ad_ext_slowtx;
>>>>
>>>> These symbols didn't exist in DPDK 2.0 but are only being added
>>>> here. So why are they being added to the 2.0 section?
>>>
>>> Yes, I think these should probably be moved.
>>>
>>>
>>>>>
>>>>> local: *;
>>>>> };
>>>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
>>>>> rte_eth_bond_free;
>>>>>
>>>>> } DPDK_2.0;
>>>>> +
>>>>> +DPDK_2.2 {
>>>>> + local
>>>>> +
>>>>> + bond_mode_8023ad_conf_get;
>>>>> + bond_mode_8023ad_setup;
>>>>> +} DPDK_2.1;
>>>>
>>>> These are marked local, as in, "not exported" which doesn't seem
>>>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
>>>> the symbol export map should look like here:
>>>
>>>
>>> These were not exported to begin with. But after versioning these
>>> functions, they are exported unless explicitly declared to be local here.
>>>
>>
>> And this does not ring any warning bells? :)
>>
>> Sorry, I was not looking at the patch as a whole. You're declaring these
>> symbols as exported with the versioning macros, eg
>>
>> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>
>> ...and then explicitly telling it to not export them by declaring local, and
>> then we wonder why it has trouble finding the symbols.
>> The versioning macros wont invent the librte_ prefix for you, you need to
>> rename the functions accordingly.
>>
>> But all this versioning gymnastics is moot anyway because you declare the
>> ABI incompatible:
>>
>> -LIBABIVER := 1
>> +LIBABIVER := 2
>>
>> This changes the library soname, so no binary compiled against the previous
>> version can possibly use it anymore. As in, by definition there can be no
>> callers of the _v20 variants after this ABI version bump.
>>
> An observation: even soname is different, just renaming .so file itself works.
>
> And this can be useful for the case:
> libx.so.1 provides functions A, B, C
> app1 compiled against libx.so.1, using only function B
>
> libx.so.1 updated only thefunction A and become libx.so.2
> app1 still can run successfully by re-naming lib to libx.so.1 (even soname is libx.so.2)
>
> But for this usage, user needs to know which function updated and is it safe or not to use this library,
> I wonder if there is an automatic way of resolving this dependency.
Erm, no. The whole point of changing soname and the physical filename is
to tell others it is incompatible with earlier versions. Yes you can
rename the file and get lucky (or not), just like you can play Russian
roulette. Neither are particularly healthy ideas.
Symbol version exists in part to allow libraries to evolve while
maintaining compatibility, but it requires careful planning and
programming. When public structs change, the structs would have to be
versioned too, and from there on it starts getting more and more
complicated.
- Panu -
>
>
>> - Panu -
>>
On Tue Nov 03 10:30:56 +0200 2015, Panu Matilainen wrote:
> On 11/03/2015 10:16 AM, Thomas Monjalon wrote:
> >2015-11-03 08:48, Panu Matilainen:
> >>On 11/02/2015 06:42 PM, Eric Kinzie wrote:
> >>>On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
> >>>>On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> >>>>>2015-10-19 08:36, Eric Kinzie:
> >>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> >>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> >>>>>> functions.
> >>>>>[...]
> >>>>>>+VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> >>>>>[...]
> >>>>>>+BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>>>>>+MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> >>>>>>+ bond_mode_8023ad_setup_v22);
> >>>>>
> >>>>>I'm sorry it doesn't work well when trying to build a combined lib:
> >>>>>
> >>>>>ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
> >>>>>
> >>>>>The symbols are OK in the .o file:
> >>>>>0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> >>>>>0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> >>>>>0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> >>>>>0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> >>>>>0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
> >>>>>
> >>>>>I don't understand the problem and I am considering disabling versioning in
> >>>>>combined library.
> >>>>>
> >>>>>Any idea?
> >>>>>
> >>>>
> >>>>The .map additions look incorrect to me:
> >>>>
> >>>>>diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>index 22bd920..7f78717 100644
> >>>>>--- a/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>+++ b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>@@ -17,6 +17,9 @@ DPDK_2.0 {
> >>>>> rte_eth_bond_slaves_get;
> >>>>> rte_eth_bond_xmit_policy_get;
> >>>>> rte_eth_bond_xmit_policy_set;
> >>>>>+ rte_eth_bond_8023ad_ext_collect;
> >>>>>+ rte_eth_bond_8023ad_ext_distrib;
> >>>>>+ rte_eth_bond_8023ad_ext_slowtx;
> >>>>
> >>>>These symbols didn't exist in DPDK 2.0 but are only being added
> >>>>here. So why are they being added to the 2.0 section?
> >>>
> >>>Yes, I think these should probably be moved.
> >>>
> >>>
> >>>>>
> >>>>> local: *;
> >>>>>};
> >>>>>@@ -27,3 +30,10 @@ DPDK_2.1 {
> >>>>> rte_eth_bond_free;
> >>>>>
> >>>>>} DPDK_2.0;
> >>>>>+
> >>>>>+DPDK_2.2 {
> >>>>>+ local
> >>>>>+
> >>>>>+ bond_mode_8023ad_conf_get;
> >>>>>+ bond_mode_8023ad_setup;
> >>>>>+} DPDK_2.1;
> >>>>
> >>>>These are marked local, as in, "not exported" which doesn't seem
> >>>>right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
> >>>>the symbol export map should look like here:
> >>>
> >>>
> >>>These were not exported to begin with. But after versioning these
> >>>functions, they are exported unless explicitly declared to be local here.
> >>>
> >>
> >>And this does not ring any warning bells? :)
> >>
> >>Sorry, I was not looking at the patch as a whole. You're declaring these
> >>symbols as exported with the versioning macros, eg
> >>
> >>BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>
> >>...and then explicitly telling it to not export them by declaring local,
> >>and then we wonder why it has trouble finding the symbols.
> >>The versioning macros wont invent the librte_ prefix for you, you need
> >>to rename the functions accordingly.
> >>
> >>But all this versioning gymnastics is moot anyway because you declare
> >>the ABI incompatible:
> >>
> >>-LIBABIVER := 1
> >>+LIBABIVER := 2
> >>
> >>This changes the library soname, so no binary compiled against the
> >>previous version can possibly use it anymore. As in, by definition there
> >>can be no callers of the _v20 variants after this ABI version bump.
> >
> >Panu, thanks for your careful review.
> >We must remove the private functions from the export map.and not bump LIBABIVER.
> >
>
> Actually the LIBABIVER bump seems appropriate since a public struct
> (rte_eth_bond_8023ad_conf) changes size.
>
> As for the rest, I'm not quite sure what the patch is trying to
> achieve. I initially assumed its trying to add a new incompatible
> version of rte_bond_mode_8023ad_setup() and
> rte_eth_bond_8023ad_conf_get() which could be a perfectly reasonable
> thing to do in some circumstances. As it is, the patch actually adds
> versioning to an internal symbol which just doesn't make sense at
> all. Symbol versioning is only meaningful for public symbols.
>
> - Panu -
It's likely I misintepreted some advice from an ealier review of
this code. The intention was to prevent an application accidentally
reading/writing past the end of a struct rte_eth_bond_8023ad_conf after
upgrading the library.
I can send another version of these patches that, instead,
adds new versions of rte_bond_mode_8023ad_setup() and
rte_eth_bond_8023ad_conf_get().
Eric
On Tue Nov 03 11:48:57 +0000 2015, Ferruh Yigit wrote:
> On Tue, Nov 03, 2015 at 01:31:45PM +0200, Panu Matilainen wrote:
> > On 11/03/2015 01:02 PM, Ferruh Yigit wrote:
> >> On Tue, Nov 03, 2015 at 08:48:16AM +0200, Panu Matilainen wrote:
> >>> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
> >>>> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
> >>>>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
> >>>>>> 2015-10-19 08:36, Eric Kinzie:
> >>>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
> >>>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
> >>>>>>> functions.
> >>>>>> [...]
> >>>>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
> >>>>>> [...]
> >>>>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>>>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
> >>>>>>> + bond_mode_8023ad_setup_v22);
> >>>>>>
> >>>>>> I'm sorry it doesn't work well when trying to build a combined lib:
> >>>>>>
> >>>>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
> >>>>>>
> >>>>>> The symbols are OK in the .o file:
> >>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
> >>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
> >>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
> >>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
> >>>>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
> >>>>>>
> >>>>>> I don't understand the problem and I am considering disabling versioning in
> >>>>>> combined library.
> >>>>>>
> >>>>>> Any idea?
> >>>>>>
> >>>>>
> >>>>> The .map additions look incorrect to me:
> >>>>>
> >>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>> index 22bd920..7f78717 100644
> >>>>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
> >>>>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
> >>>>>> rte_eth_bond_slaves_get;
> >>>>>> rte_eth_bond_xmit_policy_get;
> >>>>>> rte_eth_bond_xmit_policy_set;
> >>>>>> + rte_eth_bond_8023ad_ext_collect;
> >>>>>> + rte_eth_bond_8023ad_ext_distrib;
> >>>>>> + rte_eth_bond_8023ad_ext_slowtx;
> >>>>>
> >>>>> These symbols didn't exist in DPDK 2.0 but are only being added
> >>>>> here. So why are they being added to the 2.0 section?
> >>>>
> >>>> Yes, I think these should probably be moved.
> >>>>
> >>>>
> >>>>>>
> >>>>>> local: *;
> >>>>>> };
> >>>>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
> >>>>>> rte_eth_bond_free;
> >>>>>>
> >>>>>> } DPDK_2.0;
> >>>>>> +
> >>>>>> +DPDK_2.2 {
> >>>>>> + local
> >>>>>> +
> >>>>>> + bond_mode_8023ad_conf_get;
> >>>>>> + bond_mode_8023ad_setup;
> >>>>>> +} DPDK_2.1;
> >>>>>
> >>>>> These are marked local, as in, "not exported" which doesn't seem
> >>>>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
> >>>>> the symbol export map should look like here:
> >>>>
> >>>>
> >>>> These were not exported to begin with. But after versioning these
> >>>> functions, they are exported unless explicitly declared to be local here.
> >>>>
> >>>
> >>> And this does not ring any warning bells? :)
> >>>
> >>> Sorry, I was not looking at the patch as a whole. You're declaring these
> >>> symbols as exported with the versioning macros, eg
> >>>
> >>> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
> >>>
> >>> ...and then explicitly telling it to not export them by declaring local, and
> >>> then we wonder why it has trouble finding the symbols.
> >>> The versioning macros wont invent the librte_ prefix for you, you need to
> >>> rename the functions accordingly.
> >>>
> >>> But all this versioning gymnastics is moot anyway because you declare the
> >>> ABI incompatible:
> >>>
> >>> -LIBABIVER := 1
> >>> +LIBABIVER := 2
> >>>
> >>> This changes the library soname, so no binary compiled against the previous
> >>> version can possibly use it anymore. As in, by definition there can be no
> >>> callers of the _v20 variants after this ABI version bump.
> >>>
> >> An observation: even soname is different, just renaming .so file itself works.
> >>
> >> And this can be useful for the case:
> >> libx.so.1 provides functions A, B, C
> >> app1 compiled against libx.so.1, using only function B
> >>
> >> libx.so.1 updated only thefunction A and become libx.so.2
> >> app1 still can run successfully by re-naming lib to libx.so.1 (even soname is libx.so.2)
> >>
> >> But for this usage, user needs to know which function updated and is it safe or not to use this library,
> >> I wonder if there is an automatic way of resolving this dependency.
> >
> > Erm, no. The whole point of changing soname and the physical filename is to
> > tell others it is incompatible with earlier versions. Yes you can rename the
> > file and get lucky (or not), just like you can play Russian roulette.
> > Neither are particularly healthy ideas.
> >
> > Symbol version exists in part to allow libraries to evolve while maintaining
> > compatibility, but it requires careful planning and programming. When public
> > structs change, the structs would have to be versioned too, and from there
> > on it starts getting more and more complicated.
> >
>
> If we strictly want to prevent using old library, whenever LIBABIVER increased, we should update .map as following, right?
>
> FROM: (dpdk2.1)
> ================
> DPDK_2.0 {
> A;
> B;
> C;
> };
>
> DPDK_2.1 {
> D;
> E;
> };
>
> LIBABIVER=1
> ================
>
> TO: (dpdk2.2)
> ================
> DPDK_2.2 {
> A;
> B;
> C;
> D;
> E;
> };
>
> LIBABIVER=2
> ================
>
>
> So this won't work for anybody without luck factor, I think currently we are not doing this.
>
>
> thanks,
> ferruh
>
>
Panu, Ferruh, is there agreement on an acceptable approach to this?
Eric
On 11/20/2015 09:46 PM, Eric Kinzie wrote:
> On Tue Nov 03 11:48:57 +0000 2015, Ferruh Yigit wrote:
>> On Tue, Nov 03, 2015 at 01:31:45PM +0200, Panu Matilainen wrote:
>>> On 11/03/2015 01:02 PM, Ferruh Yigit wrote:
>>>> On Tue, Nov 03, 2015 at 08:48:16AM +0200, Panu Matilainen wrote:
>>>>> On 11/02/2015 06:42 PM, Eric Kinzie wrote:
>>>>>> On Mon Nov 02 12:23:47 +0200 2015, Panu Matilainen wrote:
>>>>>>> On 11/01/2015 08:17 PM, Thomas Monjalon wrote:
>>>>>>>> 2015-10-19 08:36, Eric Kinzie:
>>>>>>>>> Size of struct rte_eth_bond_8023ad_conf changed. Increment LIBABIVER
>>>>>>>>> and version bond_mode_8023ad_setup and bond_mode_8023ad_conf_get
>>>>>>>>> functions.
>>>>>>>> [...]
>>>>>>>>> +VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
>>>>>>>> [...]
>>>>>>>>> +BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>>>>>>>> +MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
>>>>>>>>> + bond_mode_8023ad_setup_v22);
>>>>>>>>
>>>>>>>> I'm sorry it doesn't work well when trying to build a combined lib:
>>>>>>>>
>>>>>>>> ld: libdpdk.so: version node not found for symbol bond_mode_8023ad_setup@@DPDK_2.2
>>>>>>>>
>>>>>>>> The symbols are OK in the .o file:
>>>>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup@@DPDK_2.2
>>>>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup@DPDK_2.0
>>>>>>>> 0000000000002260 g F .text 00000000000000da bond_mode_8023ad_setup_v20
>>>>>>>> 0000000000002340 g F .text 0000000000000171 bond_mode_8023ad_setup_v22
>>>>>>>> 0000000000000000 *UND* 0000000000000000 bond_mode_8023ad_setup
>>>>>>>>
>>>>>>>> I don't understand the problem and I am considering disabling versioning in
>>>>>>>> combined library.
>>>>>>>>
>>>>>>>> Any idea?
>>>>>>>>
>>>>>>>
>>>>>>> The .map additions look incorrect to me:
>>>>>>>
>>>>>>>> diff --git a/drivers/net/bonding/rte_eth_bond_version.map b/drivers/net/bonding/rte_eth_bond_version.map
>>>>>>>> index 22bd920..7f78717 100644
>>>>>>>> --- a/drivers/net/bonding/rte_eth_bond_version.map
>>>>>>>> +++ b/drivers/net/bonding/rte_eth_bond_version.map
>>>>>>>> @@ -17,6 +17,9 @@ DPDK_2.0 {
>>>>>>>> rte_eth_bond_slaves_get;
>>>>>>>> rte_eth_bond_xmit_policy_get;
>>>>>>>> rte_eth_bond_xmit_policy_set;
>>>>>>>> + rte_eth_bond_8023ad_ext_collect;
>>>>>>>> + rte_eth_bond_8023ad_ext_distrib;
>>>>>>>> + rte_eth_bond_8023ad_ext_slowtx;
>>>>>>>
>>>>>>> These symbols didn't exist in DPDK 2.0 but are only being added
>>>>>>> here. So why are they being added to the 2.0 section?
>>>>>>
>>>>>> Yes, I think these should probably be moved.
>>>>>>
>>>>>>
>>>>>>>>
>>>>>>>> local: *;
>>>>>>>> };
>>>>>>>> @@ -27,3 +30,10 @@ DPDK_2.1 {
>>>>>>>> rte_eth_bond_free;
>>>>>>>>
>>>>>>>> } DPDK_2.0;
>>>>>>>> +
>>>>>>>> +DPDK_2.2 {
>>>>>>>> + local
>>>>>>>> +
>>>>>>>> + bond_mode_8023ad_conf_get;
>>>>>>>> + bond_mode_8023ad_setup;
>>>>>>>> +} DPDK_2.1;
>>>>>>>
>>>>>>> These are marked local, as in, "not exported" which doesn't seem
>>>>>>> right. Also they're lacking the rte_eth_ prefix. AFAICS this is what
>>>>>>> the symbol export map should look like here:
>>>>>>
>>>>>>
>>>>>> These were not exported to begin with. But after versioning these
>>>>>> functions, they are exported unless explicitly declared to be local here.
>>>>>>
>>>>>
>>>>> And this does not ring any warning bells? :)
>>>>>
>>>>> Sorry, I was not looking at the patch as a whole. You're declaring these
>>>>> symbols as exported with the versioning macros, eg
>>>>>
>>>>> BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
>>>>>
>>>>> ...and then explicitly telling it to not export them by declaring local, and
>>>>> then we wonder why it has trouble finding the symbols.
>>>>> The versioning macros wont invent the librte_ prefix for you, you need to
>>>>> rename the functions accordingly.
>>>>>
>>>>> But all this versioning gymnastics is moot anyway because you declare the
>>>>> ABI incompatible:
>>>>>
>>>>> -LIBABIVER := 1
>>>>> +LIBABIVER := 2
>>>>>
>>>>> This changes the library soname, so no binary compiled against the previous
>>>>> version can possibly use it anymore. As in, by definition there can be no
>>>>> callers of the _v20 variants after this ABI version bump.
>>>>>
>>>> An observation: even soname is different, just renaming .so file itself works.
>>>>
>>>> And this can be useful for the case:
>>>> libx.so.1 provides functions A, B, C
>>>> app1 compiled against libx.so.1, using only function B
>>>>
>>>> libx.so.1 updated only thefunction A and become libx.so.2
>>>> app1 still can run successfully by re-naming lib to libx.so.1 (even soname is libx.so.2)
>>>>
>>>> But for this usage, user needs to know which function updated and is it safe or not to use this library,
>>>> I wonder if there is an automatic way of resolving this dependency.
>>>
>>> Erm, no. The whole point of changing soname and the physical filename is to
>>> tell others it is incompatible with earlier versions. Yes you can rename the
>>> file and get lucky (or not), just like you can play Russian roulette.
>>> Neither are particularly healthy ideas.
>>>
>>> Symbol version exists in part to allow libraries to evolve while maintaining
>>> compatibility, but it requires careful planning and programming. When public
>>> structs change, the structs would have to be versioned too, and from there
>>> on it starts getting more and more complicated.
>>>
>>
>> If we strictly want to prevent using old library, whenever LIBABIVER increased, we should update .map as following, right?
>>
>> FROM: (dpdk2.1)
>> ================
>> DPDK_2.0 {
>> A;
>> B;
>> C;
>> };
>>
>> DPDK_2.1 {
>> D;
>> E;
>> };
>>
>> LIBABIVER=1
>> ================
>>
>> TO: (dpdk2.2)
>> ================
>> DPDK_2.2 {
>> A;
>> B;
>> C;
>> D;
>> E;
>> };
>>
>> LIBABIVER=2
>> ================
>>
>>
>> So this won't work for anybody without luck factor, I think currently we are not doing this.
>>
>>
>> thanks,
>> ferruh
>>
>>
>
> Panu, Ferruh, is there agreement on an acceptable approach to this?
Define "this" :)
As discussed before in (and around) these:
http://dpdk.org/ml/archives/dev/2015-November/027317.html
http://dpdk.org/ml/archives/dev/2015-November/027321.html
Thomas thinks the struct modification might not even need a LIBABIVER
bump, but that all depends on the details of how the struct is used,
I've no clue about that so I wont comment further on it.
Since the added symbols seem to be intended to be private, the
versioning macro uses and .map changes should be just dropped. Only
exported symbols are to be versioned.
So ultimately the answer is, "yes, but it depends on the details" :)
- Panu -
- Panu -
@@ -41,7 +41,7 @@ CFLAGS += $(WERROR_FLAGS)
EXPORT_MAP := rte_eth_bond_version.map
-LIBABIVER := 1
+LIBABIVER := 2
#
# all source are stored in SRCS-y
@@ -39,9 +39,12 @@
#include <rte_malloc.h>
#include <rte_errno.h>
#include <rte_cycles.h>
+#include <rte_compat.h>
#include "rte_eth_bond_private.h"
+static void bond_mode_8023ad_ext_periodic_cb(void *arg);
+
#ifdef RTE_LIBRTE_BOND_DEBUG_8023AD
#define MODE4_DEBUG(fmt, ...) RTE_LOG(DEBUG, PMD, "%6u [Port %u: %s] " fmt, \
bond_dbg_get_time_diff_ms(), slave_id, \
@@ -999,7 +1002,28 @@ bond_mode_8023ad_mac_address_update(struct rte_eth_dev *bond_dev)
}
void
-bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
+bond_mode_8023ad_conf_get_v20(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf)
+{
+ struct bond_dev_private *internals = dev->data->dev_private;
+ struct mode8023ad_private *mode4 = &internals->mode4;
+ uint64_t ms_ticks = rte_get_tsc_hz() / 1000;
+
+ conf->fast_periodic_ms = mode4->fast_periodic_timeout / ms_ticks;
+ conf->slow_periodic_ms = mode4->slow_periodic_timeout / ms_ticks;
+ conf->short_timeout_ms = mode4->short_timeout / ms_ticks;
+ conf->long_timeout_ms = mode4->long_timeout / ms_ticks;
+ conf->aggregate_wait_timeout_ms = mode4->aggregate_wait_timeout / ms_ticks;
+ conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
+ conf->update_timeout_ms = mode4->update_timeout_us / 1000;
+ conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
+ conf->slowrx_cb = mode4->slowrx_cb;
+}
+
+VERSION_SYMBOL(bond_mode_8023ad_conf_get, _v20, 2.0);
+
+void
+bond_mode_8023ad_conf_get_v22(struct rte_eth_dev *dev,
struct rte_eth_bond_8023ad_conf *conf)
{
struct bond_dev_private *internals = dev->data->dev_private;
@@ -1014,10 +1038,15 @@ bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
conf->update_timeout_ms = mode4->update_timeout_us / 1000;
conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
+ conf->slowrx_cb = mode4->slowrx_cb;
}
+BIND_DEFAULT_SYMBOL(bond_mode_8023ad_conf_get, _v22, 2.2);
+MAP_STATIC_SYMBOL(void bond_mode_8023ad_conf_get(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
+ bond_mode_8023ad_conf_get_v22);
+
void
-bond_mode_8023ad_setup(struct rte_eth_dev *dev,
+bond_mode_8023ad_setup_v20(struct rte_eth_dev *dev,
struct rte_eth_bond_8023ad_conf *conf)
{
struct rte_eth_bond_8023ad_conf def_conf;
@@ -1047,6 +1076,50 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
mode4->update_timeout_us = conf->update_timeout_ms * 1000;
}
+VERSION_SYMBOL(bond_mode_8023ad_setup, _v20, 2.0);
+
+void
+bond_mode_8023ad_setup_v22(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf)
+{
+ struct rte_eth_bond_8023ad_conf def_conf;
+ struct bond_dev_private *internals = dev->data->dev_private;
+ struct mode8023ad_private *mode4 = &internals->mode4;
+ uint64_t ms_ticks = rte_get_tsc_hz() / 1000;
+
+ if (conf == NULL) {
+ conf = &def_conf;
+ conf->fast_periodic_ms = BOND_8023AD_FAST_PERIODIC_MS;
+ conf->slow_periodic_ms = BOND_8023AD_SLOW_PERIODIC_MS;
+ conf->short_timeout_ms = BOND_8023AD_SHORT_TIMEOUT_MS;
+ conf->long_timeout_ms = BOND_8023AD_LONG_TIMEOUT_MS;
+ conf->aggregate_wait_timeout_ms = BOND_8023AD_AGGREGATE_WAIT_TIMEOUT_MS;
+ conf->tx_period_ms = BOND_8023AD_TX_MACHINE_PERIOD_MS;
+ conf->rx_marker_period_ms = BOND_8023AD_RX_MARKER_PERIOD_MS;
+ conf->update_timeout_ms = BOND_MODE_8023AX_UPDATE_TIMEOUT_MS;
+ conf->slowrx_cb = NULL;
+ }
+
+ bond_mode_8023ad_stop(dev);
+
+ mode4->fast_periodic_timeout = conf->fast_periodic_ms * ms_ticks;
+ mode4->slow_periodic_timeout = conf->slow_periodic_ms * ms_ticks;
+ mode4->short_timeout = conf->short_timeout_ms * ms_ticks;
+ mode4->long_timeout = conf->long_timeout_ms * ms_ticks;
+ mode4->aggregate_wait_timeout = conf->aggregate_wait_timeout_ms * ms_ticks;
+ mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
+ mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
+ mode4->update_timeout_us = conf->update_timeout_ms * 1000;
+ mode4->slowrx_cb = conf->slowrx_cb;
+
+ if (dev->data->dev_started)
+ bond_mode_8023ad_start(dev);
+}
+
+BIND_DEFAULT_SYMBOL(bond_mode_8023ad_setup, _v22, 2.2);
+MAP_STATIC_SYMBOL(void bond_mode_8023ad_setup(struct rte_eth_dev *dev, struct rte_eth_bond_8023ad_conf *conf), \
+ bond_mode_8023ad_setup_v22);
+
int
bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
{
@@ -1062,6 +1135,13 @@ bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
int
bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
{
+ struct bond_dev_private *internals = bond_dev->data->dev_private;
+ struct mode8023ad_private *mode4 = &internals->mode4;
+
+ if (mode4->slowrx_cb)
+ return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
+ &bond_mode_8023ad_ext_periodic_cb, bond_dev);
+
return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
&bond_mode_8023ad_periodic_cb, bond_dev);
}
@@ -1069,6 +1149,13 @@ bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
void
bond_mode_8023ad_stop(struct rte_eth_dev *bond_dev)
{
+ struct bond_dev_private *internals = bond_dev->data->dev_private;
+ struct mode8023ad_private *mode4 = &internals->mode4;
+
+ if (mode4->slowrx_cb) {
+ rte_eal_alarm_cancel(&bond_mode_8023ad_ext_periodic_cb, bond_dev);
+ return;
+ }
rte_eal_alarm_cancel(&bond_mode_8023ad_periodic_cb, bond_dev);
}
@@ -1215,3 +1302,152 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
info->agg_port_id = port->aggregator_port_id;
return 0;
}
+
+int
+rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled)
+{
+ struct rte_eth_dev *bond_dev;
+ struct bond_dev_private *internals;
+ struct mode8023ad_private *mode4;
+ struct port *port;
+
+ if (rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+ return -EINVAL;
+
+ bond_dev = &rte_eth_devices[port_id];
+
+ if (!bond_dev->data->dev_started)
+ return -EINVAL;
+
+ internals = bond_dev->data->dev_private;
+ if (find_slave_by_id(internals->active_slaves,
+ internals->active_slave_count, slave_id) ==
+ internals->active_slave_count)
+ return -EINVAL;
+
+ mode4 = &internals->mode4;
+ if (mode4->slowrx_cb == NULL)
+ return -EINVAL;
+
+ port = &mode_8023ad_ports[slave_id];
+
+ if (enabled)
+ ACTOR_STATE_SET(port, COLLECTING);
+ else
+ ACTOR_STATE_CLR(port, COLLECTING);
+
+ return 0;
+}
+
+int
+rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled)
+{
+ struct rte_eth_dev *bond_dev;
+ struct bond_dev_private *internals;
+ struct mode8023ad_private *mode4;
+ struct port *port;
+
+ if (rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+ return -EINVAL;
+
+ bond_dev = &rte_eth_devices[port_id];
+
+ if (!bond_dev->data->dev_started)
+ return -EINVAL;
+
+ internals = bond_dev->data->dev_private;
+ if (find_slave_by_id(internals->active_slaves,
+ internals->active_slave_count, slave_id) ==
+ internals->active_slave_count)
+ return -EINVAL;
+
+ mode4 = &internals->mode4;
+ if (mode4->slowrx_cb == NULL)
+ return -EINVAL;
+
+ port = &mode_8023ad_ports[slave_id];
+
+ if (enabled)
+ ACTOR_STATE_SET(port, DISTRIBUTING);
+ else
+ ACTOR_STATE_CLR(port, DISTRIBUTING);
+
+ return 0;
+}
+
+int
+rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
+ struct rte_mbuf *lacp_pkt)
+{
+ struct rte_eth_dev *bond_dev;
+ struct bond_dev_private *internals;
+ struct mode8023ad_private *mode4;
+ struct port *port;
+
+ if (rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+ return -EINVAL;
+
+ bond_dev = &rte_eth_devices[port_id];
+
+ if (!bond_dev->data->dev_started)
+ return -EINVAL;
+
+ internals = bond_dev->data->dev_private;
+ if (find_slave_by_id(internals->active_slaves,
+ internals->active_slave_count, slave_id) ==
+ internals->active_slave_count)
+ return -EINVAL;
+
+ mode4 = &internals->mode4;
+ if (mode4->slowrx_cb == NULL)
+ return -EINVAL;
+
+ port = &mode_8023ad_ports[slave_id];
+
+ if (rte_pktmbuf_pkt_len(lacp_pkt) < sizeof(struct lacpdu_header))
+ return -EINVAL;
+
+ struct lacpdu_header *lacp;
+
+ /* only enqueue LACPDUs */
+ lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
+ if (lacp->lacpdu.subtype != SLOW_SUBTYPE_LACP)
+ return -EINVAL;
+
+ MODE4_DEBUG("sending LACP frame\n");
+
+ return rte_ring_enqueue(port->tx_ring, lacp_pkt);
+}
+
+static void
+bond_mode_8023ad_ext_periodic_cb(void *arg)
+{
+ struct rte_eth_dev *bond_dev = arg;
+ struct bond_dev_private *internals = bond_dev->data->dev_private;
+ struct mode8023ad_private *mode4 = &internals->mode4;
+ struct port *port;
+ void *pkt = NULL;
+ uint16_t i, slave_id;
+
+ for (i = 0; i < internals->active_slave_count; i++) {
+ slave_id = internals->active_slaves[i];
+ port = &mode_8023ad_ports[slave_id];
+
+ if (rte_ring_dequeue(port->rx_ring, &pkt) == 0) {
+ struct rte_mbuf *lacp_pkt = pkt;
+ struct lacpdu_header *lacp;
+
+ lacp = rte_pktmbuf_mtod(lacp_pkt,
+ struct lacpdu_header *);
+ RTE_VERIFY(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
+
+ /* This is LACP frame so pass it to rx callback.
+ * Callback is responsible for freeing mbuf.
+ */
+ mode4->slowrx_cb(slave_id, lacp_pkt);
+ }
+ }
+
+ rte_eal_alarm_set(internals->mode4.update_timeout_us,
+ bond_mode_8023ad_ext_periodic_cb, arg);
+}
@@ -64,6 +64,8 @@ extern "C" {
#define MARKER_TLV_TYPE_INFO 0x01
#define MARKER_TLV_TYPE_RESP 0x02
+typedef void (*rte_eth_bond_8023ad_ext_slowrx_fn)(uint8_t slave_id, struct rte_mbuf *lacp_pkt);
+
enum rte_bond_8023ad_selection {
UNSELECTED,
STANDBY,
@@ -157,6 +159,7 @@ struct rte_eth_bond_8023ad_conf {
uint32_t tx_period_ms;
uint32_t rx_marker_period_ms;
uint32_t update_timeout_ms;
+ rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
};
struct rte_eth_bond_8023ad_slave_info {
@@ -219,4 +222,45 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
}
#endif
+/**
+ * Configure a slave port to start collecting.
+ *
+ * @param port_id Bonding device id
+ * @param slave_id Port id of valid slave.
+ * @param enabled Non-zero when collection enabled.
+ * @return
+ * 0 - if ok
+ * -EINVAL if slave is not valid.
+ */
+int
+rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled);
+
+/**
+ * Configure a slave port to start distributing.
+ *
+ * @param port_id Bonding device id
+ * @param slave_id Port id of valid slave.
+ * @param enabled Non-zero when distribution enabled.
+ * @return
+ * 0 - if ok
+ * -EINVAL if slave is not valid.
+ */
+int
+rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled);
+
+/**
+ * LACPDU transmit path for external 802.3ad state machine. Caller retains
+ * ownership of the packet on failure.
+ *
+ * @param port_id Bonding device id
+ * @param slave_id Port ID of valid slave device.
+ * @param lacp_pkt mbuf containing LACPDU.
+ *
+ * @return
+ * 0 on success, negative value otherwise.
+ */
+int
+rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
+ struct rte_mbuf *lacp_pkt);
+
#endif /* RTE_ETH_BOND_8023AD_H_ */
@@ -39,6 +39,7 @@
#include <rte_ether.h>
#include <rte_byteorder.h>
#include <rte_atomic.h>
+#include <rte_compat.h>
#include "rte_eth_bond_8023ad.h"
@@ -173,6 +174,8 @@ struct mode8023ad_private {
uint64_t tx_period_timeout;
uint64_t rx_marker_timeout;
uint64_t update_timeout_us;
+ rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
+ uint8_t external_sm;
};
/**
@@ -211,6 +214,31 @@ bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
void
bond_mode_8023ad_setup(struct rte_eth_dev *dev,
struct rte_eth_bond_8023ad_conf *conf);
+void __vsym
+bond_mode_8023ad_setup_v20(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf);
+void __vsym
+bond_mode_8023ad_setup_v22(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf);
+
+/**
+ * @internal
+ *
+ * Retreive mode 4 configuration of bonded interface.
+ *
+ * @param dev Bonded interface
+ * @param conf Bonded interface configuration
+ */
+void
+bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf);
+void __vsym
+bond_mode_8023ad_conf_get_v20(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf);
+void __vsym
+bond_mode_8023ad_conf_get_v22(struct rte_eth_dev *dev,
+ struct rte_eth_bond_8023ad_conf *conf);
+
/**
* @internal
@@ -17,6 +17,9 @@ DPDK_2.0 {
rte_eth_bond_slaves_get;
rte_eth_bond_xmit_policy_get;
rte_eth_bond_xmit_policy_set;
+ rte_eth_bond_8023ad_ext_collect;
+ rte_eth_bond_8023ad_ext_distrib;
+ rte_eth_bond_8023ad_ext_slowtx;
local: *;
};
@@ -27,3 +30,10 @@ DPDK_2.1 {
rte_eth_bond_free;
} DPDK_2.0;
+
+DPDK_2.2 {
+ local:
+
+ bond_mode_8023ad_conf_get;
+ bond_mode_8023ad_setup;
+} DPDK_2.1;