Re: [PATCH 10/11] net/failsafe: fix sub-device ownership race

[Thomas Monjalon <thomas@monjalon.net>]

09/05/2018 15:30, Gaëtan Rivet:
> On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote:
> > Hi Gaetan
> > 
> > Regarding backporting.
> > This version should be bacported for 18.02.1.
> > There we have the new event.
> > 
> 
> Then the fixline should probably reflect this instead.
> Targetting the initial failsafe release won't work.
> 
> This patch also relies on probing_finish() being introduced, so I guess
> the plan is to backport the whole series in 18.02.1?

Yes, I will provide a backported series for 18.02.

 
> If so, I think the whole series should target the same commit id within
> this release, maybe the introduction of ownership or RTE_ETH_EVENT_NEW.
> 
> In any case, I think I recall being told to leave this to stable
> maintainers to deal with. However, I do not see the benefit of having
> a fixline if the information is meant to be discarded for someone to do
> the work again.

The information in the Fixes line shows where the bug was introduced.
It is used to do our backports (to know if a fix is relevant)
but it can be used for other purposes like identifying known issues
in a given version.

So, in short, these bugs can be fixed easily in 18.02, but probably not
worth to backport in older releases (no 17.11 backport).

> > Regarding uint32
> > The maximum port id number can be 0xffff.
> > In this case the loop will be infinite if we use uint16 to iterate over all the ports.
> 
> If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff]
> would be defined statically, and I think other issues would arise
> before our being stuck in an infinite loop?
> 
> In any case, if this had to be fixed, then there should be a
> BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of
> librte_ethdev, instead of relying on librte_ethdev users skirting
> shortfalls of the library. Anyone iterating on port IDs should expect the
> port_id type to be sufficient to hold this information.

Interesting thought.
I vote for keeping Matan's option as it is correct,
and will accept a patch in 18.08 for your option (BUILD_BUG_ON).
Maybe we should send a deprecation notice before limiting the max
number of ports to 0xfffe? Or is it ridiculous for such unlikely constraint?