netdev - Re: [PATCH] libbpf: Don't error out if getsockopt() fails for XDP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Sep 2019 17:53:21 +0000
From:   Yonghong Song <yhs@...com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        "Alexei Starovoitov" <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH] libbpf: Don't error out if getsockopt() fails for
 XDP_OPTIONS



On 9/9/19 10:46 AM, Toke Høiland-Jørgensen wrote:
> The xsk_socket__create() function fails and returns an error if it cannot
> get the XDP_OPTIONS through getsockopt(). However, support for XDP_OPTIONS
> was not added until kernel 5.3, so this means that creating XSK sockets
> always fails on older kernels.
> 
> Since the option is just used to set the zero-copy flag in the xsk struct,
> there really is no need to error out if the getsockopt() call fails.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
>   tools/lib/bpf/xsk.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> index 680e63066cf3..598e487d9ce8 100644
> --- a/tools/lib/bpf/xsk.c
> +++ b/tools/lib/bpf/xsk.c
> @@ -603,12 +603,8 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
>   
>   	optlen = sizeof(opts);
>   	err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
> -	if (err) {
> -		err = -errno;
> -		goto out_mmap_tx;
> -	}
> -
> -	xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
> +	if (!err)
> +		xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
>   
>   	if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
>   		err = xsk_setup_xdp_prog(xsk);

Since 'zc' is not used by anybody, maybe all codes 'zc' related can be 
removed? It can be added back back once there is an interface to use 'zc'?

diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index 842c4fd55859..24fa313524fb 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -65,7 +65,6 @@ struct xsk_socket {
         int xsks_map_fd;
         __u32 queue_id;
         char ifname[IFNAMSIZ];
-       bool zc;
  };

  struct xsk_nl_info {
@@ -491,7 +490,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, 
const char *ifname,
         void *rx_map = NULL, *tx_map = NULL;
         struct sockaddr_xdp sxdp = {};
         struct xdp_mmap_offsets off;
-       struct xdp_options opts;
         struct xsk_socket *xsk;
         socklen_t optlen;
         int err;
@@ -611,15 +609,6 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, 
const char *ifname,

         xsk->prog_fd = -1;

-       optlen = sizeof(opts);
-       err = getsockopt(xsk->fd, SOL_XDP, XDP_OPTIONS, &opts, &optlen);
-       if (err) {
-               err = -errno;
-               goto out_mmap_tx;
-       }
-
-       xsk->zc = opts.flags & XDP_OPTIONS_ZEROCOPY;
-
         if (!(xsk->config.libbpf_flags & 
XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
                 err = xsk_setup_xdp_prog(xsk);
                 if (err)