On Tue, 9 Jan 2018 13:30:46 +0300
Andrew Rybchenko <arybche...@solarflare.com> wrote:
> On 01/08/2018 08:45 PM, Stephen Hemminger wrote:
> > Many drivers are all doing copy/paste of the same code to atomically
> > update the link status. Reduce duplication, and allow for future
> > changes by having common function for this.
> >
> > Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> > ---
> > lib/librte_ether/rte_ethdev.c | 36 ++++++++++++++++++++++++++++++++++++
> > lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++++++++++
> > 2 files changed, 64 insertions(+)
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> > index a524af740f4a..6674500bbc4a 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -1495,6 +1495,42 @@ rte_eth_link_get_nowait(uint16_t port_id, struct
> > rte_eth_link *eth_link)
> > }
> > }
> >
> > +int
> > +_rte_eth_linkstatus_set(struct rte_eth_dev *dev,
> > + const struct rte_eth_link *new_link)
> > +{
> > + volatile uint64_t *dev_link
> > + = (volatile uint64_t *)&(dev->data->dev_link);
> > + union {
> > + uint64_t val64;
> > + struct rte_eth_link link;
> > + } orig;
> > +
> > + RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t));
> > +
> > + orig.val64 = rte_atomic64_exchange(dev_link,
> > + *(const uint64_t *)new_link);
> > +
> > + return (orig.link.link_status == new_link->link_status) ? -1 : 0;
>
> It still contradicts to: -1 if link state has changed, 0 if the same.
> BTW, it looks like the return value of the link_update callback is not
> specified (described) and not used. It explains why different drivers
> behave differently and nobody notices it.
It looks like link_status callback could be void.
The only places that use return value from set are code that wants
to log something if status changed.
