On Sat, 4 Nov 2017 18:45:31 +0900, Alexei Starovoitov wrote:
> On Fri, Nov 03, 2017 at 01:56:18PM -0700, Jakub Kicinski wrote:
> > Extend struct bpf_prog_info to contain information about program
> > being bound to a device. Since the netdev may get destroyed while
> > program still exists we need a flag to indicate the program is
> > loaded for a device, even if the device is gone.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> > Reviewed-by: Simon Horman <simon.hor...@netronome.com>
> > Reviewed-by: Quentin Monnet <quentin.mon...@netronome.com>
> > ---
> > include/linux/bpf.h | 1 +
> > include/uapi/linux/bpf.h | 6 ++++++
> > kernel/bpf/offload.c | 12 ++++++++++++
> > kernel/bpf/syscall.c | 5 +++++
> > 4 files changed, 24 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index e45d43f9ec92..98bacd0fa5cc 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -506,6 +506,7 @@ static inline int cpu_map_enqueue(struct
> > bpf_cpu_map_entry *rcpu,
> >
> > int bpf_prog_offload_compile(struct bpf_prog *prog);
> > void bpf_prog_offload_destroy(struct bpf_prog *prog);
> > +u32 bpf_prog_offload_ifindex(struct bpf_prog *prog);
> >
> > #if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
> > int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 727a3dba13e6..e92f62cf933a 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -894,6 +894,10 @@ enum sk_action {
> >
> > #define BPF_TAG_SIZE 8
> >
> > +enum bpf_prog_status {
> > + BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
> > +};
> > +
> > struct bpf_prog_info {
> > __u32 type;
> > __u32 id;
> > @@ -907,6 +911,8 @@ struct bpf_prog_info {
> > __u32 nr_map_ids;
> > __aligned_u64 map_ids;
> > char name[BPF_OBJ_NAME_LEN];
> > + __u32 ifindex;
> > + __u32 status;
>
> why status is needed?
> ifindex cannot be zero, so if it's set > 0 would mean
> that the program is bound.
Devices may come and go, independently from the lifetime of the program,
therefore there is a notion of a program which has been loaded for a
particular device but the device is gone (and therefore its ifindex is
meaningless). I tried to explain this in the commit message.
> Also would be good to have consistent name with prog_load.
> imo prog_target_ifindex is too long.
> May be call it 'ifindex' both in bpf_attr and in bpf_prog_info ?
Perhaps I'm missing something, but bpf_attr is a huge union of (mostly)
unnamed anonymous structs. I foresee that we will have to add an
ifindex member for a map command as well, therefore the prog_* prefix
seems prudent. Should I go back to prog_ifindex in bpf_attr?
Or perhaps should I duplicate the struct for BPF_PROG_LOAD but this
time give it a member name so we can extend it without worrying about
member name conflicts?
