Re: [PATCH bpf-next v3 5/7] samples/bpf: Add a "force" flag to XDP samples

["Maciej Fijałkowski" <maciejromanfijalkowski@gmail.com>]

On Fri, 25 Jan 2019 09:38:22 +0100
Jesper Dangaard Brouer <brouer@redhat.com> wrote:

> On Thu, 24 Jan 2019 22:47:45 +0100
> Maciej Fijalkowski <maciejromanfijalkowski@gmail.com> wrote:
> 
> > diff --git a/samples/bpf/xdp_sample_pkts_user.c
> > b/samples/bpf/xdp_sample_pkts_user.c index 5f5828ee0761..362ad35b524d 100644
> > --- a/samples/bpf/xdp_sample_pkts_user.c
> > +++ b/samples/bpf/xdp_sample_pkts_user.c
> > @@ -13,6 +13,8 @@
> >  #include <libbpf.h>
> >  #include <bpf/bpf.h>
> >  #include <sys/resource.h>
> > +#include <libgen.h>
> > +#include <linux/if_link.h>
> >  
> >  #include "perf-sys.h"
> >  #include "trace_helpers.h"
> > @@ -21,12 +23,13 @@
> >  static int pmu_fds[MAX_CPUS], if_idx;
> >  static struct perf_event_mmap_page *headers[MAX_CPUS];
> >  static char *if_name;
> > +static __u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> >  
> >  static int do_attach(int idx, int fd, const char *name)
> >  {
> >  	int err;
> >  
> > -	err = bpf_set_link_xdp_fd(idx, fd, 0);
> > +	err = bpf_set_link_xdp_fd(idx, fd, xdp_flags);
> >  	if (err < 0)
> >  		printf("ERROR: failed to attach program to %s\n", name);
> >  
> > @@ -98,21 +101,42 @@ static void sig_handler(int signo)
> >  	exit(0);
> >  }
> >  
> > +static void usage(const char *prog)
> > +{
> > +	fprintf(stderr,
> > +		"%s: %s [OPTS] IFINDEX\n\n"
>                                ^^^^^^^
> The usage help text is wrong, this sample takes the interface name as input.
>
It can take ifindex as well as ifname actually as you're saying later in this
mail - so I'm going to fix this message to "(...) IFNAME/IFINDEX" in v4.
Hope that makes sense?
> 
> > +		"OPTS:\n"
> > +		"    -F    force loading prog\n",
> > +		__func__, prog);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >  	struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
> >  	struct bpf_prog_load_attr prog_load_attr = {
> >  		.prog_type	= BPF_PROG_TYPE_XDP,
> >  	};
> > +	const char *optstr = "F";
> > +	int prog_fd, map_fd, opt;
> >  	struct bpf_object *obj;
> >  	struct bpf_map *map;
> > -	int prog_fd, map_fd;
> >  	char filename[256];
> >  	int ret, err, i;
> >  	int numcpus;
> >  
> > -	if (argc < 2) {
> > -		printf("Usage: %s <ifname>\n", argv[0]);
>                                   ^^^^^^^^
> Old help text says <ifname>
> 
> > +	while ((opt = getopt(argc, argv, optstr)) != -1) {
> > +		switch (opt) {
> > +		case 'F':
> > +			xdp_flags &= ~XDP_FLAGS_UPDATE_IF_NOEXIST;
> > +			break;
> > +		default:
> > +			usage(basename(argv[0]));
> > +			return 1;
> > +		}
> > +	}
> > +
> > +	if (optind == argc) {
> > +		usage(basename(argv[0]));
> >  		return 1;
> >  	}
> >  
> > @@ -143,16 +167,16 @@ int main(int argc, char **argv)
> >  	}
> >  	map_fd = bpf_map__fd(map);
> >  
> > -	if_idx = if_nametoindex(argv[1]);
> > +	if_idx = if_nametoindex(argv[optind]);
> 
> The we convert the ifname to ifindex.
> 
> >  	if (!if_idx)
> > -		if_idx = strtoul(argv[1], NULL, 0);
> > +		if_idx = strtoul(argv[optind], NULL, 0);
> >  
> 
> We do support falling back to using this as an ifindex.
> 
> 
> >  	if (!if_idx) {
> >  		fprintf(stderr, "Invalid ifname\n");
> >  		return 1;
> >  	}
> > -	if_name = argv[1];
> > -	err = do_attach(if_idx, prog_fd, argv[1]);
> > +	if_name = argv[optind];
> > +	err = do_attach(if_idx, prog_fd, if_name);
> >  	if (err)
> >  		return err;
> 
>