[PATCH bpf 0/3] AF_XDP small fixes

[PATCH bpf 0/3] AF_XDP small fixes

[Maciej Fijalkowski]

Hi,

This time three no-brainers, one is a fix that I sent among with
bpf_link for AF_XDP set where John suggested that it should land in bpf
tree.

Other is the missing munmap on xdpsock and some ancient function
declaration removal.

Thanks!
Maciej

Maciej Fijalkowski (3):
  xsk: remove dangling function declaration from header file
  samples: bpf: add missing munmap in xdpsock
  libbpf: clear map_info before each bpf_obj_get_info_by_fd

 include/linux/netdevice.h  | 2 --
 samples/bpf/xdpsock_user.c | 2 ++
 tools/lib/bpf/xsk.c        | 5 +++--
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH bpf 1/3] xsk: remove dangling function declaration from header file

[Maciej Fijalkowski]

xdp_umem_query() is dead for a long time, drop the declaration from
include/linux/netdevice.h

Fixes: c9b47cc1fabc ("xsk: fix bug when trying to use both copy and zero-copy on one queue id")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 include/linux/netdevice.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6cef47b76cc6..226303976310 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3932,8 +3932,6 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
 int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
 u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode);
 
-int xdp_umem_query(struct net_device *dev, u16 queue_id);
-
 int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
 int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
 int dev_forward_skb_nomtu(struct net_device *dev, struct sk_buff *skb);
-- 
2.20.1

Re: [PATCH bpf 1/3] xsk: remove dangling function declaration from header file

[Björn Töpel]

On 2021-03-03 16:51, Maciej Fijalkowski wrote:
> xdp_umem_query() is dead for a long time, drop the declaration from
> include/linux/netdevice.h
> 
> Fixes: c9b47cc1fabc ("xsk: fix bug when trying to use both copy and zero-copy on one queue id")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Acked-by: Björn Töpel <bjorn.topel@intel.com>

> ---
>   include/linux/netdevice.h | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 6cef47b76cc6..226303976310 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3932,8 +3932,6 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
>   int bpf_xdp_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
>   u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode);
>   
> -int xdp_umem_query(struct net_device *dev, u16 queue_id);
> -
>   int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
>   int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
>   int dev_forward_skb_nomtu(struct net_device *dev, struct sk_buff *skb);
> 

[PATCH bpf 2/3] samples: bpf: add missing munmap in xdpsock

[Maciej Fijalkowski]

We mmap the umem region, but we never munmap it.
Add the missing call at the end of the cleanup.

Fixes: 3945b37a975d ("samples/bpf: use hugepages in xdpsock app")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 samples/bpf/xdpsock_user.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index db0cb73513a5..1e2a1105d0e6 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -1699,5 +1699,7 @@ int main(int argc, char **argv)
 
 	xdpsock_cleanup();
 
+	munmap(bufs, NUM_FRAMES * opt_xsk_frame_size);
+
 	return 0;
 }
-- 
2.20.1

Re: [PATCH bpf 2/3] samples: bpf: add missing munmap in xdpsock

[Björn Töpel]

On 2021-03-03 16:51, Maciej Fijalkowski wrote:
> We mmap the umem region, but we never munmap it.
> Add the missing call at the end of the cleanup.
> 
> Fixes: 3945b37a975d ("samples/bpf: use hugepages in xdpsock app")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

Acked-by: Björn Töpel <bjorn.topel@intel.com>

> ---
>   samples/bpf/xdpsock_user.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
> index db0cb73513a5..1e2a1105d0e6 100644
> --- a/samples/bpf/xdpsock_user.c
> +++ b/samples/bpf/xdpsock_user.c
> @@ -1699,5 +1699,7 @@ int main(int argc, char **argv)
>   
>   	xdpsock_cleanup();
>   
> +	munmap(bufs, NUM_FRAMES * opt_xsk_frame_size);
> +
>   	return 0;
>   }
> 

[PATCH bpf 3/3] libbpf: clear map_info before each bpf_obj_get_info_by_fd

[Maciej Fijalkowski]

xsk_lookup_bpf_maps, based on prog_fd, looks whether current prog has a
reference to XSKMAP. BPF prog can include insns that work on various BPF
maps and this is covered by iterating through map_ids.

The bpf_map_info that is passed to bpf_obj_get_info_by_fd for filling
needs to be cleared at each iteration, so that it doesn't contain any
outdated fields and that is currently missing in the function of
interest.

To fix that, zero-init map_info via memset before each
bpf_obj_get_info_by_fd call.

Also, since the area of this code is touched, in general strcmp is
considered harmful, so let's convert it to strncmp and provide the
length of the array name from current map_info.

Last but not least, do s/continue/break/ once we have found the xsks_map
to terminate the search.

Fixes: 5750902a6e9b ("libbpf: proper XSKMAP cleanup")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
 tools/lib/bpf/xsk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index ffbb588724d8..e56b2d76efc2 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -610,15 +610,16 @@ static int xsk_lookup_bpf_maps(struct xsk_socket *xsk)
 		if (fd < 0)
 			continue;
 
+		memset(&map_info, 0, map_len);
 		err = bpf_obj_get_info_by_fd(fd, &map_info, &map_len);
 		if (err) {
 			close(fd);
 			continue;
 		}
 
-		if (!strcmp(map_info.name, "xsks_map")) {
+		if (!strncmp(map_info.name, "xsks_map", strlen(map_info.name))) {
 			ctx->xsks_map_fd = fd;
-			continue;
+			break;
 		}
 
 		close(fd);
-- 
2.20.1

Re: [PATCH bpf 3/3] libbpf: clear map_info before each bpf_obj_get_info_by_fd

[Björn Töpel]

On 2021-03-03 16:51, Maciej Fijalkowski wrote:
> xsk_lookup_bpf_maps, based on prog_fd, looks whether current prog has a
> reference to XSKMAP. BPF prog can include insns that work on various BPF
> maps and this is covered by iterating through map_ids.
> 
> The bpf_map_info that is passed to bpf_obj_get_info_by_fd for filling
> needs to be cleared at each iteration, so that it doesn't contain any
> outdated fields and that is currently missing in the function of
> interest.
> 
> To fix that, zero-init map_info via memset before each
> bpf_obj_get_info_by_fd call.
> 
> Also, since the area of this code is touched, in general strcmp is
> considered harmful, so let's convert it to strncmp and provide the
> length of the array name from current map_info.
> 
> Last but not least, do s/continue/break/ once we have found the xsks_map
> to terminate the search.
> 
> Fixes: 5750902a6e9b ("libbpf: proper XSKMAP cleanup")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>   tools/lib/bpf/xsk.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index ffbb588724d8..e56b2d76efc2 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -610,15 +610,16 @@ static int xsk_lookup_bpf_maps(struct xsk_socket *xsk)
>   		if (fd < 0)
>   			continue;
>   
> +		memset(&map_info, 0, map_len);
>   		err = bpf_obj_get_info_by_fd(fd, &map_info, &map_len);
>   		if (err) {
>   			close(fd);
>   			continue;
>   		}
>   
> -		if (!strcmp(map_info.name, "xsks_map")) {
> +		if (!strncmp(map_info.name, "xsks_map", strlen(map_info.name))) {

This should be sizeof(map_info.name) and not strlen, otherwise it's kind
of useless! :-P


Björn

>   			ctx->xsks_map_fd = fd;
> -			continue;
> +			break;
>   		}
>   
>   		close(fd);
>