lib: add get/set link settings interface

There are link settings parameters available from PMD drivers level
 which are currently not exposed to the user via consistent interface.
 When interface is available for system level those information can
 be acquired with 'ethtool DEVNAME' (ioctl: ETHTOOL_SLINKSETTINGS/
 ETHTOOL_GLINKSETTINGS). There are use cases where
 physical interface is passthrough to dpdk driver and is not available
 from system level. Information provided by ioctl carries information
 useful for link auto negotiation settings among others.

Signed-off-by: Marek Pazdan <mpazdan@arista.com>
---
 .mailmap                   |   1 +
 lib/ethdev/ethdev_driver.h |  33 ++++++
 lib/ethdev/rte_ethdev.c    |  26 +++++
 lib/ethdev/rte_ethdev.h    | 215 +++++++++++++++++++++++++++++++++++--
 lib/ethdev/version.map     |   3 +
 5 files changed, 270 insertions(+), 8 deletions(-)
  

On Wed, Apr 03, 2024 at 06:40:24AM -0700, Marek Pazdan wrote:
>  There are link settings parameters available from PMD drivers level
>  which are currently not exposed to the user via consistent interface.
>  When interface is available for system level those information can
>  be acquired with 'ethtool DEVNAME' (ioctl: ETHTOOL_SLINKSETTINGS/
>  ETHTOOL_GLINKSETTINGS). There are use cases where
>  physical interface is passthrough to dpdk driver and is not available
>  from system level. Information provided by ioctl carries information
>  useful for link auto negotiation settings among others.
> 
> Signed-off-by: Marek Pazdan <mpazdan@arista.com>
> ---
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 147257d6a2..66aad925d0 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -335,7 +335,7 @@ struct rte_eth_stats {
>  __extension__
>  struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */
>  	uint32_t link_speed;        /**< RTE_ETH_SPEED_NUM_ */
> -	uint16_t link_duplex  : 1;  /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */
> +	uint16_t link_duplex  : 2;  /**< RTE_ETH_LINK_[HALF/FULL/UNKNOWN]_DUPLEX */
>  	uint16_t link_autoneg : 1;  /**< RTE_ETH_LINK_[AUTONEG/FIXED] */
>  	uint16_t link_status  : 1;  /**< RTE_ETH_LINK_[DOWN/UP] */
>  };

this breaks the abi. David does libabigail pick this up i wonder?
  

Hello Tyler, Marek,

On Wed, Apr 3, 2024 at 6:49 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> On Wed, Apr 03, 2024 at 06:40:24AM -0700, Marek Pazdan wrote:
> >  There are link settings parameters available from PMD drivers level
> >  which are currently not exposed to the user via consistent interface.
> >  When interface is available for system level those information can
> >  be acquired with 'ethtool DEVNAME' (ioctl: ETHTOOL_SLINKSETTINGS/
> >  ETHTOOL_GLINKSETTINGS). There are use cases where
> >  physical interface is passthrough to dpdk driver and is not available
> >  from system level. Information provided by ioctl carries information
> >  useful for link auto negotiation settings among others.
> >
> > Signed-off-by: Marek Pazdan <mpazdan@arista.com>
> > ---
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > index 147257d6a2..66aad925d0 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -335,7 +335,7 @@ struct rte_eth_stats {
> >  __extension__
> >  struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */
> >       uint32_t link_speed;        /**< RTE_ETH_SPEED_NUM_ */
> > -     uint16_t link_duplex  : 1;  /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */
> > +     uint16_t link_duplex  : 2;  /**< RTE_ETH_LINK_[HALF/FULL/UNKNOWN]_DUPLEX */
> >       uint16_t link_autoneg : 1;  /**< RTE_ETH_LINK_[AUTONEG/FIXED] */
> >       uint16_t link_status  : 1;  /**< RTE_ETH_LINK_[DOWN/UP] */
> >  };
>
> this breaks the abi. David does libabigail pick this up i wonder?
>

Yes, the CI flagged it.

Looking at the UNH report (in patchwork):
http://mails.dpdk.org/archives/test-report/2024-April/631222.html

1 function with some indirect sub-type change:

[C] 'function int rte_eth_link_get(uint16_t, rte_eth_link*)' at
rte_ethdev.c:2972:1 has some indirect sub-type changes:
parameter 2 of type 'rte_eth_link*' has sub-type changes:
in pointed to type 'struct rte_eth_link' at rte_ethdev.h:336:1:
type size hasn't changed
2 data member changes:
'uint16_t link_autoneg' offset changed from 33 to 34 (in bits) (by +1 bits)
'uint16_t link_status' offset changed from 34 to 35 (in bits) (by +1 bits)

Error: ABI issue reported for abidiff --suppr
/home-local/jenkins-local/jenkins-agent/workspace/Generic-DPDK-Compile-ABI
at 3/dpdk/devtools/libabigail.abignore --no-added-syms --headers-dir1
reference/usr/local/include --headers-dir2
build_install/usr/local/include
reference/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.0
build_install/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.2
ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged
this as a potential issue).


GHA would have caught it too, but the documentation generation failed
before reaching the ABI check.
http://mails.dpdk.org/archives/test-report/2024-April/631086.html
  

On Thu, Apr 04, 2024 at 09:09:40AM +0200, David Marchand wrote:
> Hello Tyler, Marek,
> 
> On Wed, Apr 3, 2024 at 6:49 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > On Wed, Apr 03, 2024 at 06:40:24AM -0700, Marek Pazdan wrote:
> > >  There are link settings parameters available from PMD drivers level
> > >  which are currently not exposed to the user via consistent interface.
> > >  When interface is available for system level those information can
> > >  be acquired with 'ethtool DEVNAME' (ioctl: ETHTOOL_SLINKSETTINGS/
> > >  ETHTOOL_GLINKSETTINGS). There are use cases where
> > >  physical interface is passthrough to dpdk driver and is not available
> > >  from system level. Information provided by ioctl carries information
> > >  useful for link auto negotiation settings among others.
> > >
> > > Signed-off-by: Marek Pazdan <mpazdan@arista.com>
> > > ---
> > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > > index 147257d6a2..66aad925d0 100644
> > > --- a/lib/ethdev/rte_ethdev.h
> > > +++ b/lib/ethdev/rte_ethdev.h
> > > @@ -335,7 +335,7 @@ struct rte_eth_stats {
> > >  __extension__
> > >  struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */
> > >       uint32_t link_speed;        /**< RTE_ETH_SPEED_NUM_ */
> > > -     uint16_t link_duplex  : 1;  /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */
> > > +     uint16_t link_duplex  : 2;  /**< RTE_ETH_LINK_[HALF/FULL/UNKNOWN]_DUPLEX */
> > >       uint16_t link_autoneg : 1;  /**< RTE_ETH_LINK_[AUTONEG/FIXED] */
> > >       uint16_t link_status  : 1;  /**< RTE_ETH_LINK_[DOWN/UP] */
> > >  };
> >
> > this breaks the abi. David does libabigail pick this up i wonder?
> >
> 
> Yes, the CI flagged it.
> 
> Looking at the UNH report (in patchwork):
> http://mails.dpdk.org/archives/test-report/2024-April/631222.html

i'm jealous we don't have libabigail on windows, so helpfull.

> 
> 1 function with some indirect sub-type change:
> 
> [C] 'function int rte_eth_link_get(uint16_t, rte_eth_link*)' at
> rte_ethdev.c:2972:1 has some indirect sub-type changes:
> parameter 2 of type 'rte_eth_link*' has sub-type changes:
> in pointed to type 'struct rte_eth_link' at rte_ethdev.h:336:1:
> type size hasn't changed
> 2 data member changes:
> 'uint16_t link_autoneg' offset changed from 33 to 34 (in bits) (by +1 bits)
> 'uint16_t link_status' offset changed from 34 to 35 (in bits) (by +1 bits)
> 
> Error: ABI issue reported for abidiff --suppr
> /home-local/jenkins-local/jenkins-agent/workspace/Generic-DPDK-Compile-ABI
> at 3/dpdk/devtools/libabigail.abignore --no-added-syms --headers-dir1
> reference/usr/local/include --headers-dir2
> build_install/usr/local/include
> reference/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.0
> build_install/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.2
> ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged
> this as a potential issue).
> 
> 
> GHA would have caught it too, but the documentation generation failed
> before reaching the ABI check.
> http://mails.dpdk.org/archives/test-report/2024-April/631086.html
> 
> 
> -- 
> David Marchand
  

On Thu, Apr 04, 2024 at 05:55:18PM -0700, Tyler Retzlaff wrote:
> On Thu, Apr 04, 2024 at 09:09:40AM +0200, David Marchand wrote:
> > Hello Tyler, Marek,
> > 
> > On Wed, Apr 3, 2024 at 6:49 PM Tyler Retzlaff
> > <roretzla@linux.microsoft.com> wrote:
> > >
> > > On Wed, Apr 03, 2024 at 06:40:24AM -0700, Marek Pazdan wrote:
> > > >  There are link settings parameters available from PMD drivers level
> > > >  which are currently not exposed to the user via consistent interface.
> > > >  When interface is available for system level those information can
> > > >  be acquired with 'ethtool DEVNAME' (ioctl: ETHTOOL_SLINKSETTINGS/
> > > >  ETHTOOL_GLINKSETTINGS). There are use cases where
> > > >  physical interface is passthrough to dpdk driver and is not available
> > > >  from system level. Information provided by ioctl carries information
> > > >  useful for link auto negotiation settings among others.
> > > >
> > > > Signed-off-by: Marek Pazdan <mpazdan@arista.com>
> > > > ---
> > > > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> > > > index 147257d6a2..66aad925d0 100644
> > > > --- a/lib/ethdev/rte_ethdev.h
> > > > +++ b/lib/ethdev/rte_ethdev.h
> > > > @@ -335,7 +335,7 @@ struct rte_eth_stats {
> > > >  __extension__
> > > >  struct __rte_aligned(8) rte_eth_link { /**< aligned for atomic64 read/write */
> > > >       uint32_t link_speed;        /**< RTE_ETH_SPEED_NUM_ */
> > > > -     uint16_t link_duplex  : 1;  /**< RTE_ETH_LINK_[HALF/FULL]_DUPLEX */
> > > > +     uint16_t link_duplex  : 2;  /**< RTE_ETH_LINK_[HALF/FULL/UNKNOWN]_DUPLEX */
> > > >       uint16_t link_autoneg : 1;  /**< RTE_ETH_LINK_[AUTONEG/FIXED] */
> > > >       uint16_t link_status  : 1;  /**< RTE_ETH_LINK_[DOWN/UP] */
> > > >  };
> > >
> > > this breaks the abi. David does libabigail pick this up i wonder?
> > >
> > 
> > Yes, the CI flagged it.
> > 
> > Looking at the UNH report (in patchwork):
> > http://mails.dpdk.org/archives/test-report/2024-April/631222.html
> 
> i'm jealous we don't have libabigail on windows, so helpfull.

s/ll/l/ end of day bah.

> 
> > 
> > 1 function with some indirect sub-type change:
> > 
> > [C] 'function int rte_eth_link_get(uint16_t, rte_eth_link*)' at
> > rte_ethdev.c:2972:1 has some indirect sub-type changes:
> > parameter 2 of type 'rte_eth_link*' has sub-type changes:
> > in pointed to type 'struct rte_eth_link' at rte_ethdev.h:336:1:
> > type size hasn't changed
> > 2 data member changes:
> > 'uint16_t link_autoneg' offset changed from 33 to 34 (in bits) (by +1 bits)
> > 'uint16_t link_status' offset changed from 34 to 35 (in bits) (by +1 bits)
> > 
> > Error: ABI issue reported for abidiff --suppr
> > /home-local/jenkins-local/jenkins-agent/workspace/Generic-DPDK-Compile-ABI
> > at 3/dpdk/devtools/libabigail.abignore --no-added-syms --headers-dir1
> > reference/usr/local/include --headers-dir2
> > build_install/usr/local/include
> > reference/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.0
> > build_install/usr/local/lib/x86_64-linux-gnu/librte_ethdev.so.24.2
> > ABIDIFF_ABI_CHANGE, this change requires a review (abidiff flagged
> > this as a potential issue).
> > 
> > 
> > GHA would have caught it too, but the documentation generation failed
> > before reaching the ABI check.
> > http://mails.dpdk.org/archives/test-report/2024-April/631086.html
> > 
> > 
> > -- 
> > David Marchand
  

On Fri, Apr 5, 2024 at 2:55 AM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
> On Thu, Apr 04, 2024 at 09:09:40AM +0200, David Marchand wrote:
> > On Wed, Apr 3, 2024 at 6:49 PM Tyler Retzlaff
> > > this breaks the abi. David does libabigail pick this up i wonder?
> >
> > Yes, the CI flagged it.
> >
> > Looking at the UNH report (in patchwork):
> > http://mails.dpdk.org/archives/test-report/2024-April/631222.html
>
> i'm jealous we don't have libabigail on windows, so helpful.

libabigail is written in C++ and relies on the elfutils and libxml2 libraries.
I am unclear about what binary format is used in Windows... so I am
not sure how much work would be required to have it on Windows.

That's more something to discuss with Dodji :-).
  

Hello,

> On Fri, Apr 5, 2024 at 2:55 AM Tyler Retzlaff

[...]

>> i'm jealous we don't have libabigail on windows, so helpful.

Heh, thank you for the kind words.

David Marchand <david.marchand@redhat.com> writes:

[...]

> libabigail is written in C++ and relies on the elfutils and libxml2
> libraries.

That is correct.  Thank you for chiming in, David.

> I am unclear about what binary format is used in Windows... so I am
> not sure how much work would be required to have it on Windows.

So for a little bit of context, libabigail constructs its own internal
representation (IR) of the artifacts relevant to perform the analysis.
The middle-end of the system then performs the needed analysis on that
IR.

The binary format itself it handled by a front-end.  Today, we have
front-ends that reads the ELF format and constructs the IR and hands it
over to the middle end.

One could very well imagine a new front-end that knows how to read the
Portable Executable (PE) format that is used on Windows, along with its
accompanying debug information.

Luckily, I've written articles[1][2] that explains how libabigail
recently switched to having a multi-front-end architecture that we have
used to support alternative debug information formats like CTF[3] and
BTF.

In light of that, adding a new PE front-end would be "just work to do",
I would think.  And I would obviously not be opposed to helping such a
project and merging the result in the end.

As for any feature request, I would encourage potentially interested
parties to file an "enhancement request" on the bug tracker of the
project[4].

> That's more something to discuss with Dodji :-).

Thanks for the plug ;-)

I hope this helps.

[1]: https://developers.redhat.com/articles/2023/01/05/libabigail-multiple-debugging-formats#the_resulting_multi_front_end_architecture
[2]: https://www.redhat.com/en/blog/bpf-type-format-support-libabigail-23
[3]: https://lwn.net/Articles/795384/
[4]: https://sourceware.org/bugzilla/enter_bug.cgi?product=libabigail

Cheers,