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, 郁雷 <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
Date: Tue, 22 Dec 2020 18:24:58 -0800 [thread overview]
Message-ID: <20201222182458.4651c564@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <4a9cab3660503483fd683c89c84787a7a1b492b1.camel@mendozajonas.com>
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@bytedance.com>
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.
next prev parent reply other threads:[~2020-12-23 2:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-20 12:39 [PATCH] net/ncsi: Use real net-device for response handler John Wang
2020-12-22 6:13 ` Joel Stanley
2020-12-22 18:38 ` Samuel Mendoza-Jonas
2020-12-23 2:24 ` Jakub Kicinski [this message]
2020-12-23 5:29 ` [External] " John Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201222182458.4651c564@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=gwshan@linux.vnet.ibm.com \
--cc=joel@jms.id.au \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=sam@mendozajonas.com \
--cc=wangzhiqiang.bj@bytedance.com \
--cc=xuxiaohan@bytedance.com \
--cc=yulei.sh@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).