Re: [PATCH] net/ncsi: Use real net-device for response handler

From: Jakub Kicinski
Date: Tue Dec 22 2020 - 21:33:34 EST


On Tue, 22 Dec 2020 10:38:21 -0800 Samuel Mendoza-Jonas wrote:
> On Tue, 2020-12-22 at 06:13 +0000, Joel Stanley wrote:
> > On Sun, 20 Dec 2020 at 12:40, John Wang wrote:
> > > When aggregating ncsi interfaces and dedicated interfaces to bond
> > > interfaces, the ncsi response handler will use the wrong net device
> > > to
> > > find ncsi_dev, so that the ncsi interface will not work properly.
> > > Here, we use the net device registered to packet_type to fix it.
> > >
> > > Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
> > > Signed-off-by: John Wang <wangzhiqiang.bj@xxxxxxxxxxxxx>

This sounds like exactly the case for which orig_dev was introduced.
I think you should use the orig_dev argument, rather than pt->dev.

Can you test if that works?

> > Can you show me how to reproduce this?
> >
> > I don't know the ncsi or net code well enough to know if this is the
> > correct fix. If you are confident it is correct then I have no
> > objections.
>
> This looks like it is probably right; pt->dev will be the original
> device from ncsi_register_dev(), if a response comes in to
> ncsi_rcv_rsp() associated with a different device then the driver will
> fail to find the correct ncsi_dev_priv. An example of the broken case
> would be good to see though.

From the description sounds like the case is whenever the ncsi
interface is in a bond, the netdev from the second argument is
the bond not the interface from which the frame came. It should
be possible to repro even with only one interface on the system,
create a bond or a team and add the ncsi interface to it.

Does that make sense? I'm likely missing the subtleties here.