diff mbox series

[RFC,bpf-next,3/4] libbpf: move xdp program removal to libbpf

Message ID 20190603131907.13395-4-maciej.fijalkowski@intel.com
State RFC
Delegated to: BPF Maintainers
Headers show
Series [RFC,bpf-next,1/4] libbpf: fill the AF_XDP fill queue before bind() call | expand

Commit Message

Maciej Fijalkowski June 3, 2019, 1:19 p.m. UTC 
Since xsk support in libbpf loads the xdp program interface, make it
also responsible for its removal. Store the prog id in xsk_socket_config
so when removing the program we are still able to compare the current
program id with the id from the attachment time and make a decision
onward.

While at it, remove the socket/umem in xdpsock's error path.

Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 samples/bpf/xdpsock_user.c | 33 ++++++++++-----------------------
 tools/lib/bpf/xsk.c        | 32 ++++++++++++++++++++++++++++++++
 tools/lib/bpf/xsk.h        |  1 +
 3 files changed, 43 insertions(+), 23 deletions(-)

Comments

Björn Töpel June 4, 2019, 8:07 a.m. UTC | #1 
On 2019-06-03 15:19, Maciej Fijalkowski wrote:
> Since xsk support in libbpf loads the xdp program interface, make it
> also responsible for its removal. Store the prog id in xsk_socket_config
> so when removing the program we are still able to compare the current
> program id with the id from the attachment time and make a decision
> onward.
> 
> While at it, remove the socket/umem in xdpsock's error path.
>

We're loading a new, or reusing an existing XDP program at socket
creation, but tearing it down at *socket delete* is explicitly left to
the application.

For a per-queue XDP program (tied to the socket), this kind cleanup would
make sense.

The intention with the libbpf AF_XDP support was to leave the XDP
handling to whatever XDP orchestration process availble. It's not part
of libbpf. For convenience, *loading/lookup of the XDP program* was
added even though this was an asymmetry.

For the sample application, this makes sense, but for larger/real
applications?

OTOH I like the idea of a scoped cleanup "when all sockets are gone",
the XDP program + maps are removed.

> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>   samples/bpf/xdpsock_user.c | 33 ++++++++++-----------------------
>   tools/lib/bpf/xsk.c        | 32 ++++++++++++++++++++++++++++++++
>   tools/lib/bpf/xsk.h        |  1 +
>   3 files changed, 43 insertions(+), 23 deletions(-)
> 
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index e9dceb09b6d1..123862b16dd4 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -68,7 +68,6 @@ static int opt_queue;
>   static int opt_poll;
>   static int opt_interval = 1;
>   static u32 opt_xdp_bind_flags;
> -static __u32 prog_id;
>   
>   struct xsk_umem_info {
>   	struct xsk_ring_prod fq;
> @@ -170,22 +169,6 @@ static void *poller(void *arg)
>   	return NULL;
>   }
>   
> -static void remove_xdp_program(void)
> -{
> -	__u32 curr_prog_id = 0;
> -
> -	if (bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags)) {
> -		printf("bpf_get_link_xdp_id failed\n");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (prog_id == curr_prog_id)
> -		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> -	else if (!curr_prog_id)
> -		printf("couldn't find a prog id on a given interface\n");
> -	else
> -		printf("program on interface changed, not removing\n");
> -}
> -
>   static void int_exit(int sig)
>   {
>   	struct xsk_umem *umem = xsks[0]->umem->umem;
> @@ -195,7 +178,6 @@ static void int_exit(int sig)
>   	dump_stats();
>   	xsk_socket__delete(xsks[0]->xsk);
>   	(void)xsk_umem__delete(umem);
> -	remove_xdp_program();
>   
>   	exit(EXIT_SUCCESS);
>   }
> @@ -206,7 +188,16 @@ static void __exit_with_error(int error, const char *file, const char *func,
>   	fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func,
>   		line, error, strerror(error));
>   	dump_stats();
> -	remove_xdp_program();
> +
> +	if (xsks[0]->xsk)
> +		xsk_socket__delete(xsks[0]->xsk);
> +
> +	if (xsks[0]->umem) {
> +		struct xsk_umem *umem = xsks[0]->umem->umem;
> +
> +		(void)xsk_umem__delete(umem);
> +	}
> +
>   	exit(EXIT_FAILURE);
>   }
>   
> @@ -312,10 +303,6 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
>   	if (ret)
>   		exit_with_error(-ret);
>   
> -	ret = bpf_get_link_xdp_id(opt_ifindex, &prog_id, opt_xdp_flags);
> -	if (ret)
> -		exit_with_error(-ret);
> -
>   	return xsk;
>   }
>   
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 514ab3fb06f4..e28bedb0b078 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -259,6 +259,8 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
>   static int xsk_load_xdp_prog(struct xsk_socket *xsk)
>   {
>   	static const int log_buf_size = 16 * 1024;
> +	struct bpf_prog_info info = {};
> +	__u32 info_len = sizeof(info);
>   	char log_buf[log_buf_size];
>   	int err, prog_fd;
>   
> @@ -321,6 +323,14 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
>   		return err;
>   	}
>   
> +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> +	if (err) {
> +		pr_warning("can't get prog info - %s\n", strerror(errno));
> +		close(prog_fd);
> +		return err;
> +	}
> +	xsk->config.prog_id = info.id;
> +
>   	xsk->prog_fd = prog_fd;
>   	return 0;
>   }
> @@ -483,6 +493,25 @@ static int xsk_set_bpf_maps(struct xsk_socket *xsk)
>   	return err;
>   }
>   
> +static void xsk_remove_xdp_prog(struct xsk_socket *xsk)
> +{
> +	__u32 prog_id = xsk->config.prog_id;
> +	__u32 curr_prog_id = 0;
> +	int err;
> +
> +	err = bpf_get_link_xdp_id(xsk->ifindex, &curr_prog_id,
> +				  xsk->config.xdp_flags);
> +	if (err)
> +		return;
> +
> +	if (prog_id == curr_prog_id)
> +		bpf_set_link_xdp_fd(xsk->ifindex, -1, xsk->config.xdp_flags);
> +	else if (!curr_prog_id)
> +		pr_warning("couldn't find a prog id on a given interface\n");
> +	else
> +		pr_warning("program on interface changed, not removing\n");
> +}
> +
>   static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
>   {
>   	__u32 prog_id = 0;
> @@ -506,6 +535,7 @@ static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
>   		err = xsk_lookup_bpf_maps(xsk);
>   		if (err)
>   			goto out_load;
> +		xsk->config.prog_id = prog_id;
>   	}
>   
>   	err = xsk_set_bpf_maps(xsk);
> @@ -744,6 +774,8 @@ void xsk_socket__delete(struct xsk_socket *xsk)
>   
>   	}
>   
> +	xsk_remove_xdp_prog(xsk);
> +
>   	xsk->umem->refcount--;
>   	/* Do not close an fd that also has an associated umem connected
>   	 * to it.
> diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> index 82ea71a0f3ec..e1b23e9432c9 100644
> --- a/tools/lib/bpf/xsk.h
> +++ b/tools/lib/bpf/xsk.h
> @@ -186,6 +186,7 @@ struct xsk_socket_config {
>   	__u32 tx_size;
>   	__u32 libbpf_flags;
>   	__u32 xdp_flags;
> +	__u32 prog_id;
>   	__u16 bind_flags;
>   };
>   
>
Maciej Fijalkowski June 4, 2019, 3:06 p.m. UTC | #2 
On Tue, 4 Jun 2019 10:07:25 +0200
Björn Töpel <bjorn.topel@intel.com> wrote:

> 
> On 2019-06-03 15:19, Maciej Fijalkowski wrote:
> > Since xsk support in libbpf loads the xdp program interface, make it
> > also responsible for its removal. Store the prog id in xsk_socket_config
> > so when removing the program we are still able to compare the current
> > program id with the id from the attachment time and make a decision
> > onward.
> > 
> > While at it, remove the socket/umem in xdpsock's error path.
> >
> 
> We're loading a new, or reusing an existing XDP program at socket
> creation, but tearing it down at *socket delete* is explicitly left to
> the application.

Are you describing here the old behavior?

> 
> For a per-queue XDP program (tied to the socket), this kind cleanup would
> make sense.
> 
> The intention with the libbpf AF_XDP support was to leave the XDP
> handling to whatever XDP orchestration process availble. It's not part
> of libbpf. For convenience, *loading/lookup of the XDP program* was
> added even though this was an asymmetry.

Hmmm ok and I tried to make it symmetric :p 

> 
> For the sample application, this makes sense, but for larger/real
> applications?
>

Tough questions on those real apps!


> OTOH I like the idea of a scoped cleanup "when all sockets are gone",
> the XDP program + maps are removed.

That's happening with patch 4 included from this set (in case it gets fixed :))

> 
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> >   samples/bpf/xdpsock_user.c | 33 ++++++++++-----------------------
> >   tools/lib/bpf/xsk.c        | 32 ++++++++++++++++++++++++++++++++
> >   tools/lib/bpf/xsk.h        |  1 +
> >   3 files changed, 43 insertions(+), 23 deletions(-)
> > 
> > diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> > index e9dceb09b6d1..123862b16dd4 100644
> > --- a/samples/bpf/xdpsock_user.c
> > +++ b/samples/bpf/xdpsock_user.c
> > @@ -68,7 +68,6 @@ static int opt_queue;
> >   static int opt_poll;
> >   static int opt_interval = 1;
> >   static u32 opt_xdp_bind_flags;
> > -static __u32 prog_id;
> >   
> >   struct xsk_umem_info {
> >   	struct xsk_ring_prod fq;
> > @@ -170,22 +169,6 @@ static void *poller(void *arg)
> >   	return NULL;
> >   }
> >   
> > -static void remove_xdp_program(void)
> > -{
> > -	__u32 curr_prog_id = 0;
> > -
> > -	if (bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags)) {
> > -		printf("bpf_get_link_xdp_id failed\n");
> > -		exit(EXIT_FAILURE);
> > -	}
> > -	if (prog_id == curr_prog_id)
> > -		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> > -	else if (!curr_prog_id)
> > -		printf("couldn't find a prog id on a given interface\n");
> > -	else
> > -		printf("program on interface changed, not removing\n");
> > -}
> > -
> >   static void int_exit(int sig)
> >   {
> >   	struct xsk_umem *umem = xsks[0]->umem->umem;
> > @@ -195,7 +178,6 @@ static void int_exit(int sig)
> >   	dump_stats();
> >   	xsk_socket__delete(xsks[0]->xsk);
> >   	(void)xsk_umem__delete(umem);
> > -	remove_xdp_program();
> >   
> >   	exit(EXIT_SUCCESS);
> >   }
> > @@ -206,7 +188,16 @@ static void __exit_with_error(int error, const char *file, const char *func,
> >   	fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func,
> >   		line, error, strerror(error));
> >   	dump_stats();
> > -	remove_xdp_program();
> > +
> > +	if (xsks[0]->xsk)
> > +		xsk_socket__delete(xsks[0]->xsk);
> > +
> > +	if (xsks[0]->umem) {
> > +		struct xsk_umem *umem = xsks[0]->umem->umem;
> > +
> > +		(void)xsk_umem__delete(umem);
> > +	}
> > +
> >   	exit(EXIT_FAILURE);
> >   }
> >   
> > @@ -312,10 +303,6 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
> >   	if (ret)
> >   		exit_with_error(-ret);
> >   
> > -	ret = bpf_get_link_xdp_id(opt_ifindex, &prog_id, opt_xdp_flags);
> > -	if (ret)
> > -		exit_with_error(-ret);
> > -
> >   	return xsk;
> >   }
> >   
> > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > index 514ab3fb06f4..e28bedb0b078 100644
> > --- a/tools/lib/bpf/xsk.c
> > +++ b/tools/lib/bpf/xsk.c
> > @@ -259,6 +259,8 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
> >   static int xsk_load_xdp_prog(struct xsk_socket *xsk)
> >   {
> >   	static const int log_buf_size = 16 * 1024;
> > +	struct bpf_prog_info info = {};
> > +	__u32 info_len = sizeof(info);
> >   	char log_buf[log_buf_size];
> >   	int err, prog_fd;
> >   
> > @@ -321,6 +323,14 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
> >   		return err;
> >   	}
> >   
> > +	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > +	if (err) {
> > +		pr_warning("can't get prog info - %s\n", strerror(errno));
> > +		close(prog_fd);
> > +		return err;
> > +	}
> > +	xsk->config.prog_id = info.id;
> > +
> >   	xsk->prog_fd = prog_fd;
> >   	return 0;
> >   }
> > @@ -483,6 +493,25 @@ static int xsk_set_bpf_maps(struct xsk_socket *xsk)
> >   	return err;
> >   }
> >   
> > +static void xsk_remove_xdp_prog(struct xsk_socket *xsk)
> > +{
> > +	__u32 prog_id = xsk->config.prog_id;
> > +	__u32 curr_prog_id = 0;
> > +	int err;
> > +
> > +	err = bpf_get_link_xdp_id(xsk->ifindex, &curr_prog_id,
> > +				  xsk->config.xdp_flags);
> > +	if (err)
> > +		return;
> > +
> > +	if (prog_id == curr_prog_id)
> > +		bpf_set_link_xdp_fd(xsk->ifindex, -1, xsk->config.xdp_flags);
> > +	else if (!curr_prog_id)
> > +		pr_warning("couldn't find a prog id on a given interface\n");
> > +	else
> > +		pr_warning("program on interface changed, not removing\n");
> > +}
> > +
> >   static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
> >   {
> >   	__u32 prog_id = 0;
> > @@ -506,6 +535,7 @@ static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
> >   		err = xsk_lookup_bpf_maps(xsk);
> >   		if (err)
> >   			goto out_load;
> > +		xsk->config.prog_id = prog_id;
> >   	}
> >   
> >   	err = xsk_set_bpf_maps(xsk);
> > @@ -744,6 +774,8 @@ void xsk_socket__delete(struct xsk_socket *xsk)
> >   
> >   	}
> >   
> > +	xsk_remove_xdp_prog(xsk);
> > +
> >   	xsk->umem->refcount--;
> >   	/* Do not close an fd that also has an associated umem connected
> >   	 * to it.
> > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> > index 82ea71a0f3ec..e1b23e9432c9 100644
> > --- a/tools/lib/bpf/xsk.h
> > +++ b/tools/lib/bpf/xsk.h
> > @@ -186,6 +186,7 @@ struct xsk_socket_config {
> >   	__u32 tx_size;
> >   	__u32 libbpf_flags;
> >   	__u32 xdp_flags;
> > +	__u32 prog_id;
> >   	__u16 bind_flags;
> >   };
> >   
> >
Björn Töpel June 5, 2019, 9:03 a.m. UTC | #3 
On Tue, 4 Jun 2019 at 17:07, Maciej Fijalkowski
<maciejromanfijalkowski@gmail.com> wrote:
>
> On Tue, 4 Jun 2019 10:07:25 +0200
> Björn Töpel <bjorn.topel@intel.com> wrote:
>
> >
> > On 2019-06-03 15:19, Maciej Fijalkowski wrote:
> > > Since xsk support in libbpf loads the xdp program interface, make it
> > > also responsible for its removal. Store the prog id in xsk_socket_config
> > > so when removing the program we are still able to compare the current
> > > program id with the id from the attachment time and make a decision
> > > onward.
> > >
> > > While at it, remove the socket/umem in xdpsock's error path.
> > >
> >
> > We're loading a new, or reusing an existing XDP program at socket
> > creation, but tearing it down at *socket delete* is explicitly left to
> > the application.
>
> Are you describing here the old behavior?
>
> >
> > For a per-queue XDP program (tied to the socket), this kind cleanup would
> > make sense.
> >
> > The intention with the libbpf AF_XDP support was to leave the XDP
> > handling to whatever XDP orchestration process availble. It's not part
> > of libbpf. For convenience, *loading/lookup of the XDP program* was
> > added even though this was an asymmetry.
>
> Hmmm ok and I tried to make it symmetric :p
>

Thought a bit more about this, and I think you're right here. It
should be symmetric! Please continue this work! (But keep in mind that
it might go away if/once per-queue programs appear. :-P)

> >
> > For the sample application, this makes sense, but for larger/real
> > applications?
> >
>
> Tough questions on those real apps!
>

:-D

>
> > OTOH I like the idea of a scoped cleanup "when all sockets are gone",
> > the XDP program + maps are removed.
>
> That's happening with patch 4 included from this set (in case it gets fixed :))
>

Ok!

Björn

> >
> > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > > ---
> > >   samples/bpf/xdpsock_user.c | 33 ++++++++++-----------------------
> > >   tools/lib/bpf/xsk.c        | 32 ++++++++++++++++++++++++++++++++
> > >   tools/lib/bpf/xsk.h        |  1 +
> > >   3 files changed, 43 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> > > index e9dceb09b6d1..123862b16dd4 100644
> > > --- a/samples/bpf/xdpsock_user.c
> > > +++ b/samples/bpf/xdpsock_user.c
> > > @@ -68,7 +68,6 @@ static int opt_queue;
> > >   static int opt_poll;
> > >   static int opt_interval = 1;
> > >   static u32 opt_xdp_bind_flags;
> > > -static __u32 prog_id;
> > >
> > >   struct xsk_umem_info {
> > >     struct xsk_ring_prod fq;
> > > @@ -170,22 +169,6 @@ static void *poller(void *arg)
> > >     return NULL;
> > >   }
> > >
> > > -static void remove_xdp_program(void)
> > > -{
> > > -   __u32 curr_prog_id = 0;
> > > -
> > > -   if (bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags)) {
> > > -           printf("bpf_get_link_xdp_id failed\n");
> > > -           exit(EXIT_FAILURE);
> > > -   }
> > > -   if (prog_id == curr_prog_id)
> > > -           bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
> > > -   else if (!curr_prog_id)
> > > -           printf("couldn't find a prog id on a given interface\n");
> > > -   else
> > > -           printf("program on interface changed, not removing\n");
> > > -}
> > > -
> > >   static void int_exit(int sig)
> > >   {
> > >     struct xsk_umem *umem = xsks[0]->umem->umem;
> > > @@ -195,7 +178,6 @@ static void int_exit(int sig)
> > >     dump_stats();
> > >     xsk_socket__delete(xsks[0]->xsk);
> > >     (void)xsk_umem__delete(umem);
> > > -   remove_xdp_program();
> > >
> > >     exit(EXIT_SUCCESS);
> > >   }
> > > @@ -206,7 +188,16 @@ static void __exit_with_error(int error, const char *file, const char *func,
> > >     fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func,
> > >             line, error, strerror(error));
> > >     dump_stats();
> > > -   remove_xdp_program();
> > > +
> > > +   if (xsks[0]->xsk)
> > > +           xsk_socket__delete(xsks[0]->xsk);
> > > +
> > > +   if (xsks[0]->umem) {
> > > +           struct xsk_umem *umem = xsks[0]->umem->umem;
> > > +
> > > +           (void)xsk_umem__delete(umem);
> > > +   }
> > > +
> > >     exit(EXIT_FAILURE);
> > >   }
> > >
> > > @@ -312,10 +303,6 @@ static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
> > >     if (ret)
> > >             exit_with_error(-ret);
> > >
> > > -   ret = bpf_get_link_xdp_id(opt_ifindex, &prog_id, opt_xdp_flags);
> > > -   if (ret)
> > > -           exit_with_error(-ret);
> > > -
> > >     return xsk;
> > >   }
> > >
> > > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > > index 514ab3fb06f4..e28bedb0b078 100644
> > > --- a/tools/lib/bpf/xsk.c
> > > +++ b/tools/lib/bpf/xsk.c
> > > @@ -259,6 +259,8 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
> > >   static int xsk_load_xdp_prog(struct xsk_socket *xsk)
> > >   {
> > >     static const int log_buf_size = 16 * 1024;
> > > +   struct bpf_prog_info info = {};
> > > +   __u32 info_len = sizeof(info);
> > >     char log_buf[log_buf_size];
> > >     int err, prog_fd;
> > >
> > > @@ -321,6 +323,14 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
> > >             return err;
> > >     }
> > >
> > > +   err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
> > > +   if (err) {
> > > +           pr_warning("can't get prog info - %s\n", strerror(errno));
> > > +           close(prog_fd);
> > > +           return err;
> > > +   }
> > > +   xsk->config.prog_id = info.id;
> > > +
> > >     xsk->prog_fd = prog_fd;
> > >     return 0;
> > >   }
> > > @@ -483,6 +493,25 @@ static int xsk_set_bpf_maps(struct xsk_socket *xsk)
> > >     return err;
> > >   }
> > >
> > > +static void xsk_remove_xdp_prog(struct xsk_socket *xsk)
> > > +{
> > > +   __u32 prog_id = xsk->config.prog_id;
> > > +   __u32 curr_prog_id = 0;
> > > +   int err;
> > > +
> > > +   err = bpf_get_link_xdp_id(xsk->ifindex, &curr_prog_id,
> > > +                             xsk->config.xdp_flags);
> > > +   if (err)
> > > +           return;
> > > +
> > > +   if (prog_id == curr_prog_id)
> > > +           bpf_set_link_xdp_fd(xsk->ifindex, -1, xsk->config.xdp_flags);
> > > +   else if (!curr_prog_id)
> > > +           pr_warning("couldn't find a prog id on a given interface\n");
> > > +   else
> > > +           pr_warning("program on interface changed, not removing\n");
> > > +}
> > > +
> > >   static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
> > >   {
> > >     __u32 prog_id = 0;
> > > @@ -506,6 +535,7 @@ static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
> > >             err = xsk_lookup_bpf_maps(xsk);
> > >             if (err)
> > >                     goto out_load;
> > > +           xsk->config.prog_id = prog_id;
> > >     }
> > >
> > >     err = xsk_set_bpf_maps(xsk);
> > > @@ -744,6 +774,8 @@ void xsk_socket__delete(struct xsk_socket *xsk)
> > >
> > >     }
> > >
> > > +   xsk_remove_xdp_prog(xsk);
> > > +
> > >     xsk->umem->refcount--;
> > >     /* Do not close an fd that also has an associated umem connected
> > >      * to it.
> > > diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
> > > index 82ea71a0f3ec..e1b23e9432c9 100644
> > > --- a/tools/lib/bpf/xsk.h
> > > +++ b/tools/lib/bpf/xsk.h
> > > @@ -186,6 +186,7 @@ struct xsk_socket_config {
> > >     __u32 tx_size;
> > >     __u32 libbpf_flags;
> > >     __u32 xdp_flags;
> > > +   __u32 prog_id;
> > >     __u16 bind_flags;
> > >   };
> > >
> > >
>
diff mbox series

Patch

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index e9dceb09b6d1..123862b16dd4 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -68,7 +68,6 @@  static int opt_queue;
 static int opt_poll;
 static int opt_interval = 1;
 static u32 opt_xdp_bind_flags;
-static __u32 prog_id;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
@@ -170,22 +169,6 @@  static void *poller(void *arg)
 	return NULL;
 }
 
-static void remove_xdp_program(void)
-{
-	__u32 curr_prog_id = 0;
-
-	if (bpf_get_link_xdp_id(opt_ifindex, &curr_prog_id, opt_xdp_flags)) {
-		printf("bpf_get_link_xdp_id failed\n");
-		exit(EXIT_FAILURE);
-	}
-	if (prog_id == curr_prog_id)
-		bpf_set_link_xdp_fd(opt_ifindex, -1, opt_xdp_flags);
-	else if (!curr_prog_id)
-		printf("couldn't find a prog id on a given interface\n");
-	else
-		printf("program on interface changed, not removing\n");
-}
-
 static void int_exit(int sig)
 {
 	struct xsk_umem *umem = xsks[0]->umem->umem;
@@ -195,7 +178,6 @@  static void int_exit(int sig)
 	dump_stats();
 	xsk_socket__delete(xsks[0]->xsk);
 	(void)xsk_umem__delete(umem);
-	remove_xdp_program();
 
 	exit(EXIT_SUCCESS);
 }
@@ -206,7 +188,16 @@  static void __exit_with_error(int error, const char *file, const char *func,
 	fprintf(stderr, "%s:%s:%i: errno: %d/\"%s\"\n", file, func,
 		line, error, strerror(error));
 	dump_stats();
-	remove_xdp_program();
+
+	if (xsks[0]->xsk)
+		xsk_socket__delete(xsks[0]->xsk);
+
+	if (xsks[0]->umem) {
+		struct xsk_umem *umem = xsks[0]->umem->umem;
+
+		(void)xsk_umem__delete(umem);
+	}
+
 	exit(EXIT_FAILURE);
 }
 
@@ -312,10 +303,6 @@  static struct xsk_socket_info *xsk_configure_socket(struct xsk_umem_info *umem)
 	if (ret)
 		exit_with_error(-ret);
 
-	ret = bpf_get_link_xdp_id(opt_ifindex, &prog_id, opt_xdp_flags);
-	if (ret)
-		exit_with_error(-ret);
-
 	return xsk;
 }
 
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 514ab3fb06f4..e28bedb0b078 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -259,6 +259,8 @@  int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
 static int xsk_load_xdp_prog(struct xsk_socket *xsk)
 {
 	static const int log_buf_size = 16 * 1024;
+	struct bpf_prog_info info = {};
+	__u32 info_len = sizeof(info);
 	char log_buf[log_buf_size];
 	int err, prog_fd;
 
@@ -321,6 +323,14 @@  static int xsk_load_xdp_prog(struct xsk_socket *xsk)
 		return err;
 	}
 
+	err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
+	if (err) {
+		pr_warning("can't get prog info - %s\n", strerror(errno));
+		close(prog_fd);
+		return err;
+	}
+	xsk->config.prog_id = info.id;
+
 	xsk->prog_fd = prog_fd;
 	return 0;
 }
@@ -483,6 +493,25 @@  static int xsk_set_bpf_maps(struct xsk_socket *xsk)
 	return err;
 }
 
+static void xsk_remove_xdp_prog(struct xsk_socket *xsk)
+{
+	__u32 prog_id = xsk->config.prog_id;
+	__u32 curr_prog_id = 0;
+	int err;
+
+	err = bpf_get_link_xdp_id(xsk->ifindex, &curr_prog_id,
+				  xsk->config.xdp_flags);
+	if (err)
+		return;
+
+	if (prog_id == curr_prog_id)
+		bpf_set_link_xdp_fd(xsk->ifindex, -1, xsk->config.xdp_flags);
+	else if (!curr_prog_id)
+		pr_warning("couldn't find a prog id on a given interface\n");
+	else
+		pr_warning("program on interface changed, not removing\n");
+}
+
 static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
 {
 	__u32 prog_id = 0;
@@ -506,6 +535,7 @@  static int xsk_setup_xdp_prog(struct xsk_socket *xsk)
 		err = xsk_lookup_bpf_maps(xsk);
 		if (err)
 			goto out_load;
+		xsk->config.prog_id = prog_id;
 	}
 
 	err = xsk_set_bpf_maps(xsk);
@@ -744,6 +774,8 @@  void xsk_socket__delete(struct xsk_socket *xsk)
 
 	}
 
+	xsk_remove_xdp_prog(xsk);
+
 	xsk->umem->refcount--;
 	/* Do not close an fd that also has an associated umem connected
 	 * to it.
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 82ea71a0f3ec..e1b23e9432c9 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -186,6 +186,7 @@  struct xsk_socket_config {
 	__u32 tx_size;
 	__u32 libbpf_flags;
 	__u32 xdp_flags;
+	__u32 prog_id;
 	__u16 bind_flags;
 };