Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
@ 2019-06-12 16:14 Maxim Mikityanskiy
  2019-06-12 18:39 ` Björn Töpel
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Maxim Mikityanskiy @ 2019-06-12 16:14 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Björn Töpel
  Cc: bpf, netdev, David S. Miller, Saeed Mahameed, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Martin KaFai Lau,
	Song Liu, Yonghong Song, Maxim Mikityanskiy

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.

 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);
-- 
2.19.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  2019-06-12 16:14 [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed Maxim Mikityanskiy
@ 2019-06-12 18:39 ` Björn Töpel
  2019-06-12 21:15 ` Jakub Kicinski
  2019-07-10 11:16 ` Maxim Mikityanskiy
  2 siblings, 0 replies; 11+ messages in thread
From: Björn Töpel @ 2019-06-12 18:39 UTC (permalink / raw)
  To: Maxim Mikityanskiy, Alexei Starovoitov, Daniel Borkmann
  Cc: bpf, netdev, David S. Miller, Saeed Mahameed, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend, Martin KaFai Lau,
	Song Liu, Yonghong Song

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);
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  2019-06-12 16:14 [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed Maxim Mikityanskiy
  2019-06-12 18:39 ` Björn Töpel
@ 2019-06-12 21:15 ` Jakub Kicinski
  2019-06-13  9:04   ` Björn Töpel
  2019-07-10 11:16 ` Maxim Mikityanskiy
  2 siblings, 1 reply; 11+ messages in thread
From: Jakub Kicinski @ 2019-06-12 21:15 UTC (permalink / raw)
  To: Maxim Mikityanskiy
  Cc: Alexei Starovoitov, Daniel Borkmann, Björn Töpel, bpf,
	netdev, David S. Miller, Saeed Mahameed, Jesper Dangaard Brouer,
	John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song

On Wed, 12 Jun 2019 16:14:18 +0000, 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.
> 
>  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;

This will mask the error if program is installed with different flags.

You driver should do nothing is program installation state did not
change.  I.e.:

	if (!!prog == !!oldprog)

You can't remove the active -> active check anyway, this change should
make no difference.

>  	}
>  
>  	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  2019-06-12 21:15 ` Jakub Kicinski
@ 2019-06-13  9:04   ` Björn Töpel
  2019-06-13 17:26     ` Jakub Kicinski
  0 siblings, 1 reply; 11+ messages in thread
From: Björn Töpel @ 2019-06-13  9:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Maxim Mikityanskiy, Alexei Starovoitov, Daniel Borkmann,
	Björn Töpel, bpf, netdev, David S. Miller,
	Saeed Mahameed, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song

On Wed, 12 Jun 2019 at 23:15, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Wed, 12 Jun 2019 16:14:18 +0000, 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.
> >
> >  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;
>
> This will mask the error if program is installed with different flags.
>

Hmm, probably missing something, but I fail to see how the error is
being masked? This is to catch the NULL-to-NULL case early.

> You driver should do nothing is program installation state did not
> change.  I.e.:
>
>         if (!!prog == !!oldprog)
>
> You can't remove the active -> active check anyway, this change should
> make no difference.
>
> >       }
> >
> >       err = dev_xdp_install(dev, bpf_op, extack, flags, prog);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  2019-06-13  9:04   ` Björn Töpel
@ 2019-06-13 17:26     ` Jakub Kicinski
  0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2019-06-13 17:26 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Maxim Mikityanskiy, Alexei Starovoitov, Daniel Borkmann,
	Björn Töpel, bpf, netdev, David S. Miller,
	Saeed Mahameed, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song

On Thu, 13 Jun 2019 11:04:45 +0200, Björn Töpel wrote:
> > > +     } else {
> > > +             if (!__dev_xdp_query(dev, bpf_op, query))
> > > +                     return 0;  
> >
> > This will mask the error if program is installed with different flags.
> >  
> 
> Hmm, probably missing something, but I fail to see how the error is
> being masked? This is to catch the NULL-to-NULL case early.

You're right, it shouldn't matter.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  2019-06-12 16:14 [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed Maxim Mikityanskiy
  2019-06-12 18:39 ` Björn Töpel
  2019-06-12 21:15 ` Jakub Kicinski
@ 2019-07-10 11:16 ` Maxim Mikityanskiy
  2019-07-12 15:44   ` Daniel Borkmann
  2 siblings, 1 reply; 11+ messages in thread
From: Maxim Mikityanskiy @ 2019-07-10 11:16 UTC (permalink / raw)
  To: Alexei Starovoitov, David S. Miller
  Cc: Daniel Borkmann, Björn Töpel, bpf, netdev,
	Saeed Mahameed, Jakub Kicinski, Jesper Dangaard Brouer,
	John Fastabend, Martin KaFai Lau, Song Liu, Yonghong Song

On 2019-06-12 19: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.
> 
>   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);
> 

Alexei, so what about this patch? It's marked as "Changed Requested" in 
patchwork, but Jakub's point looks resolved - I don't see any changes 
required from my side.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed
  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
  0 siblings, 1 reply; 11+ messages in thread
From: Daniel Borkmann @ 2019-07-12 15:44 UTC (permalink / raw)
  To: Maxim Mikityanskiy, Alexei Starovoitov, David S. Miller
  Cc: Björn Töpel, bpf, netdev, Saeed Mahameed,
	Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song

On 07/10/2019 01:16 PM, Maxim Mikityanskiy wrote:
> On 2019-06-12 19: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.
>>
>>   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);
>>
> 
> Alexei, so what about this patch? It's marked as "Changed Requested" in 
> patchwork, but Jakub's point looks resolved - I don't see any changes 
> required from my side.

I believe part of Jakub's feedback was that if we make this generic that this
does not generally address the case where both prog pointers are equal (whether
NULL or non-NULL).

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH bpf-next v2] net: Don't call XDP_SETUP_PROG when nothing is changed
  2019-07-12 15:44   ` Daniel Borkmann
@ 2019-08-14 14:34     ` Maxim Mikityanskiy
  2019-08-14 17:57       ` Jonathan Lemon
                         ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Maxim Mikityanskiy @ 2019-08-14 14:34 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski
  Cc: bpf, netdev, David S. Miller, Björn Töpel,
	Saeed Mahameed, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song, Maxim Mikityanskiy

Don't uninstall an XDP program when none is installed, and don't install
an XDP program that has the same ID as the one already installed.

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 symmetrical case is possible when the user tries to set the program
that is already set.

The drivers typically perform some heavy operations on XDP_SETUP_PROG,
so they all have to handle these cases internally to return early if
they happen. 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>
---
v2 changes: Cover the case when the program is set, but isn't changed.

 net/core/dev.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 49589ed2018d..b1afafee3e2a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8126,12 +8126,15 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 		bpf_chk = generic_xdp_install;
 
 	if (fd >= 0) {
+		u32 prog_id;
+
 		if (!offload && __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG)) {
 			NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
 			return -EEXIST;
 		}
-		if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) &&
-		    __dev_xdp_query(dev, bpf_op, query)) {
+
+		prog_id = __dev_xdp_query(dev, bpf_op, query);
+		if ((flags & XDP_FLAGS_UPDATE_IF_NOEXIST) && prog_id) {
 			NL_SET_ERR_MSG(extack, "XDP program already attached");
 			return -EBUSY;
 		}
@@ -8146,6 +8149,14 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 			bpf_prog_put(prog);
 			return -EINVAL;
 		}
+
+		if (prog->aux->id == prog_id) {
+			bpf_prog_put(prog);
+			return 0;
+		}
+	} else {
+		if (!__dev_xdp_query(dev, bpf_op, query))
+			return 0;
 	}
 
 	err = dev_xdp_install(dev, bpf_op, extack, flags, prog);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v2] net: Don't call XDP_SETUP_PROG when nothing is changed
  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
  2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Lemon @ 2019-08-14 17:57 UTC (permalink / raw)
  To: Maxim Mikityanskiy
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski, bpf, netdev,
	David S. Miller, Björn Töpel, Saeed Mahameed,
	Jesper Dangaard Brouer, John Fastabend, Martin KaFai Lau,
	Song Liu, Yonghong Song



On 14 Aug 2019, at 7:34, Maxim Mikityanskiy wrote:

> Don't uninstall an XDP program when none is installed, and don't install
> an XDP program that has the same ID as the one already installed.
>
> 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 symmetrical case is possible when the user tries to set the program
> that is already set.
>
> The drivers typically perform some heavy operations on XDP_SETUP_PROG,
> so they all have to handle these cases internally to return early if
> they happen. 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>

Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v2] net: Don't call XDP_SETUP_PROG when nothing is changed
  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
  2 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2019-08-14 22:01 UTC (permalink / raw)
  To: Maxim Mikityanskiy
  Cc: Alexei Starovoitov, Daniel Borkmann, bpf, netdev,
	David S. Miller, Björn Töpel, Saeed Mahameed,
	Jesper Dangaard Brouer, John Fastabend, Martin KaFai Lau,
	Song Liu, Yonghong Song

On Wed, 14 Aug 2019 14:34:06 +0000, Maxim Mikityanskiy wrote:
> Don't uninstall an XDP program when none is installed, and don't install
> an XDP program that has the same ID as the one already installed.
> 
> 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 symmetrical case is possible when the user tries to set the program
> that is already set.
> 
> The drivers typically perform some heavy operations on XDP_SETUP_PROG,
> so they all have to handle these cases internally to return early if
> they happen. 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>

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH bpf-next v2] net: Don't call XDP_SETUP_PROG when nothing is changed
  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
  2 siblings, 0 replies; 11+ messages in thread
From: Daniel Borkmann @ 2019-08-17 21:29 UTC (permalink / raw)
  To: Maxim Mikityanskiy, Alexei Starovoitov, Jakub Kicinski
  Cc: bpf, netdev, David S. Miller, Björn Töpel,
	Saeed Mahameed, Jesper Dangaard Brouer, John Fastabend,
	Martin KaFai Lau, Song Liu, Yonghong Song

On 8/14/19 4:34 PM, Maxim Mikityanskiy wrote:
> Don't uninstall an XDP program when none is installed, and don't install
> an XDP program that has the same ID as the one already installed.
> 
> 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 symmetrical case is possible when the user tries to set the program
> that is already set.
> 
> The drivers typically perform some heavy operations on XDP_SETUP_PROG,
> so they all have to handle these cases internally to return early if
> they happen. 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>

Applied, thanks!

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 16:14 [PATCH bpf-next] net: Don't uninstall an XDP program when none is installed Maxim Mikityanskiy
2019-06-12 18:39 ` Björn Töpel
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

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 netdev@archiver.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