On Wed, 30 Jan 2019 19:12:48 +0100 Maciej Fijalkowski <maciejromanfijalkow...@gmail.com> wrote:
> On Tue, 29 Jan 2019 12:34:05 +0100 > Jesper Dangaard Brouer <bro...@redhat.com> wrote: > > > On Tue, 29 Jan 2019 09:00:00 +0100 > > Jesper Dangaard Brouer <bro...@redhat.com> wrote: > > > > > On Mon, 28 Jan 2019 20:16:11 +0100 > > > Maciej Fijalkowski <maciejromanfijalkow...@gmail.com> wrote: > > > > > > > From: Maciej Fijalkowski <maciej.fijalkow...@intel.com> > > > > > > > > Make xdp samples consistent with iproute2 behavior and set the > > > > XDP_FLAGS_UPDATE_IF_NOEXIST by default when setting the xdp program on > > > > interface. Provide an option for user to force the program loading, > > > > which as a result will not include the mentioned flag in > > > > bpf_set_link_xdp_fd call. > > > > > > I like the idea, but what is the error message users get after this > > > change? > > > > $ sudo ./xdp1 mlx5p1 & > > [1] 9768 > > > > $ sudo ./xdp1 mlx5p1 > > link set xdp fd failed > > > > This error message is a little too generic to be a good user experience. > > The kernel (in dev_change_xdp_fd) will return errno -EBUSY (-16), but > > we don't use or report the return value in these sample programs. > > > > If my QA see this error message, I will still get an error report > > bugzilla that I need to spend time on investigating. Can we please > > improve this error message? > > > > If you are really cool you get inspired by (or use) libbpf_strerror() > > code avail in tools/lib/bpf/libbpf_errno.c. Default strerror(EBUSY) > > will return "Device or resource busy", but maybe we can do slightly > > better and report something more meaningful for this XDP context. > > > I'll post a v5 with libbpf_strerror() usage when bpf_set_link_xdp_fd failed in > samples but at this point it will only give us a standard "device or resource > busy" error That is a good first iteration improvement. And if QA complains, I can quickly diagnose and explain the issue, based on this generic message, without digging further and wasting more time. > , so if we need some more meaningful message that libbpf will give > us then I guess we need to define a new libbpf_errno entry (as well as entry > in > libbpf_strerror_table for this new errno value) and set the errno in > bpf_set_link_xdp_fd in case of a failure? It likely require more work do provide more meaningful messages, and I guess it is out-of-scope for your patchset. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer
