From: "Björn Töpel" <bjorn.topel@intel.com> To: Maxim Mikityanskiy <maximmi@mellanox.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Cc: "bpf@vger.kernel.org" <bpf@vger.kernel.org>, "netdev@vger.kernel.org" <netdev@vger.kernel.org>, "David S. Miller" <davem@davemloft.net>, Saeed Mahameed <saeedm@mellanox.com>, Jakub Kicinski <jakub.kicinski@netronome.com>, Jesper Dangaard Brouer <hawk@kernel.org>, John Fastabend <john.fastabend@gmail.com>, Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com> Subject: Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed Date: Wed, 12 Jun 2019 20:39:22 +0200 Message-ID: <fab294a1-ab48-cc48-8240-3ef4f1dcaf9f@intel.com> (raw) In-Reply-To: <20190612161405.24064-1-maximmi@mellanox.com> On 2019-06-12 18:14, Maxim Mikityanskiy wrote: > dev_change_xdp_fd doesn't perform any checks in case it uninstalls an > XDP program. It means that the driver's ndo_bpf can be called with > XDP_SETUP_PROG asking to set it to NULL even if it's already NULL. This > case happens if the user runs `ip link set eth0 xdp off` when there is > no XDP program attached. > > The drivers typically perform some heavy operations on XDP_SETUP_PROG, > so they all have to handle this case internally to return early if it > happens. This patch puts this check into the kernel code, so that all > drivers will benefit from it. > > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com> > --- > Björn, please take a look at this, Saeed told me you were doing > something related, but I couldn't find it. If this fix is already > covered by your work, please tell about that. > Yeah, I'm trying make the query code more generic (pull common work out from the drivers). However, my patch set is still not done (I'll try to get a v4 out this week), but your improvement is! I don't see why this should be held back because of my still not finished work. I like this. Acked-by: Björn Töpel <bjorn.topel@intel.com> > net/core/dev.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 66f7508825bd..68b3e3320ceb 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -8089,6 +8089,9 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack, > bpf_prog_put(prog); > return -EINVAL; > } > + } else { > + if (!__dev_xdp_query(dev, bpf_op, query)) > + return 0; > } > > err = dev_xdp_install(dev, bpf_op, extack, flags, prog); >
next prev parent reply index Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-12 16:14 Maxim Mikityanskiy 2019-06-12 18:39 ` Björn Töpel [this message] 2019-06-12 21:15 ` Jakub Kicinski 2019-06-13 9:04 ` Björn Töpel 2019-06-13 17:26 ` Jakub Kicinski 2019-07-10 11:16 ` Maxim Mikityanskiy 2019-07-12 15:44 ` Daniel Borkmann 2019-08-14 14:34 ` [PATCH bpf-next v2] net: Don't call XDP_SETUP_PROG when nothing is changed Maxim Mikityanskiy 2019-08-14 17:57 ` Jonathan Lemon 2019-08-14 22:01 ` Jakub Kicinski 2019-08-17 21:29 ` Daniel Borkmann
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=fab294a1-ab48-cc48-8240-3ef4f1dcaf9f@intel.com \ --to=bjorn.topel@intel.com \ --cc=ast@kernel.org \ --cc=bpf@vger.kernel.org \ --cc=daniel@iogearbox.net \ --cc=davem@davemloft.net \ --cc=hawk@kernel.org \ --cc=jakub.kicinski@netronome.com \ --cc=john.fastabend@gmail.com \ --cc=kafai@fb.com \ --cc=maximmi@mellanox.com \ --cc=netdev@vger.kernel.org \ --cc=saeedm@mellanox.com \ --cc=songliubraving@fb.com \ --cc=yhs@fb.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
Netdev Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \ netdev@vger.kernel.org public-inbox-index netdev Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.netdev AGPL code for this site: git clone https://public-inbox.org/public-inbox.git