Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 22 Dec 2020 18:24:58 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Samuel Mendoza-Jonas <sam@mendozajonas.com>
Cc:     Joel Stanley <joel@jms.id.au>,
        John Wang <wangzhiqiang.bj@bytedance.com>,
        xuxiaohan@bytedance.com,
        =?UTF-8?B?6YOB?= =?UTF-8?B?6Zu3?= <yulei.sh@bytedance.com>,
        "David S. Miller" <davem@davemloft.net>,
        Gavin Shan <gwshan@linux.vnet.ibm.com>,
        "open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net/ncsi: Use real net-device for response handler
Message-ID: <20201222182458.4651c564@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <4a9cab3660503483fd683c89c84787a7a1b492b1.camel@mendozajonas.com>
References: <20201220123957.1694-1-wangzhiqiang.bj@bytedance.com>
        <CACPK8XexOmUOdGmHCYVXVgA0z5m99XCAbixcgODSoUSRNCY+zA@mail.gmail.com>
        <4a9cab3660503483fd683c89c84787a7a1b492b1.camel@mendozajonas.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

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.
> > >=20
> > > Fixes: 138635cc27c9 ("net/ncsi: NCSI response packet handler")
> > > Signed-off-by: John Wang <wangzhiqiang.bj@bytedance.com> =20

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?
> >=20
> > 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. =20
>=20
> 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.

=46rom the description sounds like the case is whenever the ncsi
interface is in a bond, the netdev from the second argument is=20
the bond not the interface from which the frame came. It should=20
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.