Skip to content

Commit 6547f42

Browse files
committed
Merge branch 'bpf-offload-fixes'
Jakub Kicinski says: ==================== This series addresses some late comments and moves checking if program has been loaded for the correct device to the drivers. There are also some problems with net namespaces which I didn't take into consideration. On the kernel side we will now simply ignore namespace moves. Since the user space API is not reporting any namespace identification we have to remove the ifindex until a correct way of reporting is agreed upon. v2: - fix ext ack reporting for XDP (David A); - add Jiri's Ack. ==================== Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
2 parents 32a72bb + 1438019 commit 6547f42

File tree

10 files changed

+56
-110
lines changed

10 files changed

+56
-110
lines changed

drivers/net/ethernet/netronome/nfp/bpf/offload.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,14 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct bpf_prog *prog,
214214
{
215215
int err;
216216

217-
if (prog && !prog->aux->offload)
218-
return -EINVAL;
217+
if (prog) {
218+
struct bpf_dev_offload *offload = prog->aux->offload;
219+
220+
if (!offload)
221+
return -EINVAL;
222+
if (offload->netdev != nn->dp.netdev)
223+
return -EINVAL;
224+
}
219225

220226
if (prog && old_prog) {
221227
u8 cap;

include/linux/bpf.h

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -334,9 +334,8 @@ extern const struct bpf_verifier_ops tc_cls_act_analyzer_ops;
334334
extern const struct bpf_verifier_ops xdp_analyzer_ops;
335335

336336
struct bpf_prog *bpf_prog_get(u32 ufd);
337-
struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type);
338337
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
339-
struct net_device *netdev);
338+
bool attach_drv);
340339
struct bpf_prog * __must_check bpf_prog_add(struct bpf_prog *prog, int i);
341340
void bpf_prog_sub(struct bpf_prog *prog, int i);
342341
struct bpf_prog * __must_check bpf_prog_inc(struct bpf_prog *prog);
@@ -425,15 +424,9 @@ static inline struct bpf_prog *bpf_prog_get(u32 ufd)
425424
return ERR_PTR(-EOPNOTSUPP);
426425
}
427426

428-
static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
429-
enum bpf_prog_type type)
430-
{
431-
return ERR_PTR(-EOPNOTSUPP);
432-
}
433-
434427
static inline struct bpf_prog *bpf_prog_get_type_dev(u32 ufd,
435428
enum bpf_prog_type type,
436-
struct net_device *netdev)
429+
bool attach_drv)
437430
{
438431
return ERR_PTR(-EOPNOTSUPP);
439432
}
@@ -514,9 +507,14 @@ static inline int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu,
514507
}
515508
#endif /* CONFIG_BPF_SYSCALL */
516509

510+
static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
511+
enum bpf_prog_type type)
512+
{
513+
return bpf_prog_get_type_dev(ufd, type, false);
514+
}
515+
517516
int bpf_prog_offload_compile(struct bpf_prog *prog);
518517
void bpf_prog_offload_destroy(struct bpf_prog *prog);
519-
u32 bpf_prog_offload_ifindex(struct bpf_prog *prog);
520518

521519
#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
522520
int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr);

include/linux/bpf_verifier.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ static inline struct bpf_reg_state *cur_regs(struct bpf_verifier_env *env)
171171
#if defined(CONFIG_NET) && defined(CONFIG_BPF_SYSCALL)
172172
int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env);
173173
#else
174-
int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
174+
static inline int bpf_prog_offload_verifier_prep(struct bpf_verifier_env *env)
175175
{
176176
return -EOPNOTSUPP;
177177
}

include/uapi/linux/bpf.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ union bpf_attr {
262262
__u32 kern_version; /* checked when prog_type=kprobe */
263263
__u32 prog_flags;
264264
char prog_name[BPF_OBJ_NAME_LEN];
265-
__u32 prog_target_ifindex; /* ifindex of netdev to prep for */
265+
__u32 prog_ifindex; /* ifindex of netdev to prep for */
266266
};
267267

268268
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -897,10 +897,6 @@ enum sk_action {
897897

898898
#define BPF_TAG_SIZE 8
899899

900-
enum bpf_prog_status {
901-
BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
902-
};
903-
904900
struct bpf_prog_info {
905901
__u32 type;
906902
__u32 id;
@@ -914,8 +910,6 @@ struct bpf_prog_info {
914910
__u32 nr_map_ids;
915911
__aligned_u64 map_ids;
916912
char name[BPF_OBJ_NAME_LEN];
917-
__u32 ifindex;
918-
__u32 status;
919913
} __attribute__((aligned(8)));
920914

921915
struct bpf_map_info {

kernel/bpf/offload.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
1414
struct net *net = current->nsproxy->net_ns;
1515
struct bpf_dev_offload *offload;
1616

17-
if (!capable(CAP_SYS_ADMIN))
18-
return -EPERM;
17+
if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS &&
18+
attr->prog_type != BPF_PROG_TYPE_XDP)
19+
return -EINVAL;
1920

2021
if (attr->prog_flags)
2122
return -EINVAL;
@@ -28,7 +29,7 @@ int bpf_prog_offload_init(struct bpf_prog *prog, union bpf_attr *attr)
2829
init_waitqueue_head(&offload->verifier_done);
2930

3031
rtnl_lock();
31-
offload->netdev = __dev_get_by_index(net, attr->prog_target_ifindex);
32+
offload->netdev = __dev_get_by_index(net, attr->prog_ifindex);
3233
if (!offload->netdev) {
3334
rtnl_unlock();
3435
kfree(offload);
@@ -85,6 +86,10 @@ static void __bpf_prog_offload_destroy(struct bpf_prog *prog)
8586
struct bpf_dev_offload *offload = prog->aux->offload;
8687
struct netdev_bpf data = {};
8788

89+
/* Caution - if netdev is destroyed before the program, this function
90+
* will be called twice.
91+
*/
92+
8893
data.offload.prog = prog;
8994

9095
if (offload->verifier_running)
@@ -144,18 +149,6 @@ int bpf_prog_offload_compile(struct bpf_prog *prog)
144149
return bpf_prog_offload_translate(prog);
145150
}
146151

147-
u32 bpf_prog_offload_ifindex(struct bpf_prog *prog)
148-
{
149-
struct bpf_dev_offload *offload = prog->aux->offload;
150-
u32 ifindex;
151-
152-
rtnl_lock();
153-
ifindex = offload->netdev ? offload->netdev->ifindex : 0;
154-
rtnl_unlock();
155-
156-
return ifindex;
157-
}
158-
159152
const struct bpf_prog_ops bpf_offload_prog_ops = {
160153
};
161154

@@ -169,6 +162,10 @@ static int bpf_offload_notification(struct notifier_block *notifier,
169162

170163
switch (event) {
171164
case NETDEV_UNREGISTER:
165+
/* ignore namespace changes */
166+
if (netdev->reg_state != NETREG_UNREGISTERING)
167+
break;
168+
172169
list_for_each_entry_safe(offload, tmp, &bpf_prog_offload_devs,
173170
offloads) {
174171
if (offload->netdev == netdev)

kernel/bpf/syscall.c

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,30 +1057,31 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
10571057
}
10581058
EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
10591059

1060-
static bool bpf_prog_can_attach(struct bpf_prog *prog,
1061-
enum bpf_prog_type *attach_type,
1062-
struct net_device *netdev)
1060+
static bool bpf_prog_get_ok(struct bpf_prog *prog,
1061+
enum bpf_prog_type *attach_type, bool attach_drv)
10631062
{
1064-
struct bpf_dev_offload *offload = prog->aux->offload;
1063+
/* not an attachment, just a refcount inc, always allow */
1064+
if (!attach_type)
1065+
return true;
10651066

10661067
if (prog->type != *attach_type)
10671068
return false;
1068-
if (offload && offload->netdev != netdev)
1069+
if (bpf_prog_is_dev_bound(prog->aux) && !attach_drv)
10691070
return false;
10701071

10711072
return true;
10721073
}
10731074

10741075
static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
1075-
struct net_device *netdev)
1076+
bool attach_drv)
10761077
{
10771078
struct fd f = fdget(ufd);
10781079
struct bpf_prog *prog;
10791080

10801081
prog = ____bpf_prog_get(f);
10811082
if (IS_ERR(prog))
10821083
return prog;
1083-
if (attach_type && !bpf_prog_can_attach(prog, attach_type, netdev)) {
1084+
if (!bpf_prog_get_ok(prog, attach_type, attach_drv)) {
10841085
prog = ERR_PTR(-EINVAL);
10851086
goto out;
10861087
}
@@ -1093,23 +1094,13 @@ static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type,
10931094

10941095
struct bpf_prog *bpf_prog_get(u32 ufd)
10951096
{
1096-
return __bpf_prog_get(ufd, NULL, NULL);
1097+
return __bpf_prog_get(ufd, NULL, false);
10971098
}
10981099

1099-
struct bpf_prog *bpf_prog_get_type(u32 ufd, enum bpf_prog_type type)
1100-
{
1101-
struct bpf_prog *prog = __bpf_prog_get(ufd, &type, NULL);
1102-
1103-
if (!IS_ERR(prog))
1104-
trace_bpf_prog_get_type(prog);
1105-
return prog;
1106-
}
1107-
EXPORT_SYMBOL_GPL(bpf_prog_get_type);
1108-
11091100
struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
1110-
struct net_device *netdev)
1101+
bool attach_drv)
11111102
{
1112-
struct bpf_prog *prog = __bpf_prog_get(ufd, &type, netdev);
1103+
struct bpf_prog *prog = __bpf_prog_get(ufd, &type, attach_drv);
11131104

11141105
if (!IS_ERR(prog))
11151106
trace_bpf_prog_get_type(prog);
@@ -1118,7 +1109,7 @@ struct bpf_prog *bpf_prog_get_type_dev(u32 ufd, enum bpf_prog_type type,
11181109
EXPORT_SYMBOL_GPL(bpf_prog_get_type_dev);
11191110

11201111
/* last field in 'union bpf_attr' used by this command */
1121-
#define BPF_PROG_LOAD_LAST_FIELD prog_target_ifindex
1112+
#define BPF_PROG_LOAD_LAST_FIELD prog_ifindex
11221113

11231114
static int bpf_prog_load(union bpf_attr *attr)
11241115
{
@@ -1181,7 +1172,7 @@ static int bpf_prog_load(union bpf_attr *attr)
11811172
atomic_set(&prog->aux->refcnt, 1);
11821173
prog->gpl_compatible = is_gpl ? 1 : 0;
11831174

1184-
if (attr->prog_target_ifindex) {
1175+
if (attr->prog_ifindex) {
11851176
err = bpf_prog_offload_init(prog, attr);
11861177
if (err)
11871178
goto free_prog;
@@ -1625,11 +1616,6 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
16251616
return -EFAULT;
16261617
}
16271618

1628-
if (bpf_prog_is_dev_bound(prog->aux)) {
1629-
info.status |= BPF_PROG_STATUS_DEV_BOUND;
1630-
info.ifindex = bpf_prog_offload_ifindex(prog);
1631-
}
1632-
16331619
done:
16341620
if (copy_to_user(uinfo, &info, info_len) ||
16351621
put_user(info_len, &uattr->info.info_len))

net/core/dev.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7139,13 +7139,17 @@ int dev_change_xdp_fd(struct net_device *dev, struct netlink_ext_ack *extack,
71397139
__dev_xdp_attached(dev, bpf_op, NULL))
71407140
return -EBUSY;
71417141

7142-
if (bpf_op == ops->ndo_bpf)
7143-
prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
7144-
dev);
7145-
else
7146-
prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
7142+
prog = bpf_prog_get_type_dev(fd, BPF_PROG_TYPE_XDP,
7143+
bpf_op == ops->ndo_bpf);
71477144
if (IS_ERR(prog))
71487145
return PTR_ERR(prog);
7146+
7147+
if (!(flags & XDP_FLAGS_HW_MODE) &&
7148+
bpf_prog_is_dev_bound(prog->aux)) {
7149+
NL_SET_ERR_MSG(extack, "using device-bound program without HW_MODE flag is not supported");
7150+
bpf_prog_put(prog);
7151+
return -EINVAL;
7152+
}
71497153
}
71507154

71517155
err = dev_xdp_install(dev, bpf_op, extack, flags, prog);

net/sched/cls_bpf.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,15 +382,13 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
382382
{
383383
struct bpf_prog *fp;
384384
char *name = NULL;
385+
bool skip_sw;
385386
u32 bpf_fd;
386387

387388
bpf_fd = nla_get_u32(tb[TCA_BPF_FD]);
389+
skip_sw = gen_flags & TCA_CLS_FLAGS_SKIP_SW;
388390

389-
if (gen_flags & TCA_CLS_FLAGS_SKIP_SW)
390-
fp = bpf_prog_get_type_dev(bpf_fd, BPF_PROG_TYPE_SCHED_CLS,
391-
qdisc_dev(tp->q));
392-
else
393-
fp = bpf_prog_get_type(bpf_fd, BPF_PROG_TYPE_SCHED_CLS);
391+
fp = bpf_prog_get_type_dev(bpf_fd, BPF_PROG_TYPE_SCHED_CLS, skip_sw);
394392
if (IS_ERR(fp))
395393
return PTR_ERR(fp);
396394

tools/bpf/bpftool/prog.c

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
#include <string.h>
4242
#include <time.h>
4343
#include <unistd.h>
44-
#include <net/if.h>
4544
#include <sys/types.h>
4645
#include <sys/stat.h>
4746

@@ -230,21 +229,6 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
230229
info->tag[0], info->tag[1], info->tag[2], info->tag[3],
231230
info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
232231

233-
if (info->status & BPF_PROG_STATUS_DEV_BOUND) {
234-
jsonw_name(json_wtr, "dev");
235-
if (info->ifindex) {
236-
char name[IF_NAMESIZE];
237-
238-
if (!if_indextoname(info->ifindex, name))
239-
jsonw_printf(json_wtr, "\"ifindex:%d\"",
240-
info->ifindex);
241-
else
242-
jsonw_printf(json_wtr, "\"%s\"", name);
243-
} else {
244-
jsonw_printf(json_wtr, "\"unknown\"");
245-
}
246-
}
247-
248232
if (info->load_time) {
249233
char buf[32];
250234

@@ -302,21 +286,6 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
302286

303287
printf("tag ");
304288
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
305-
printf(" ");
306-
307-
if (info->status & BPF_PROG_STATUS_DEV_BOUND) {
308-
printf("dev ");
309-
if (info->ifindex) {
310-
char name[IF_NAMESIZE];
311-
312-
if (!if_indextoname(info->ifindex, name))
313-
printf("ifindex:%d ", info->ifindex);
314-
else
315-
printf("%s ", name);
316-
} else {
317-
printf("unknown ");
318-
}
319-
}
320289
printf("\n");
321290

322291
if (info->load_time) {

tools/include/uapi/linux/bpf.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ union bpf_attr {
262262
__u32 kern_version; /* checked when prog_type=kprobe */
263263
__u32 prog_flags;
264264
char prog_name[BPF_OBJ_NAME_LEN];
265-
__u32 prog_target_ifindex; /* ifindex of netdev to prep for */
265+
__u32 prog_ifindex; /* ifindex of netdev to prep for */
266266
};
267267

268268
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -897,10 +897,6 @@ enum sk_action {
897897

898898
#define BPF_TAG_SIZE 8
899899

900-
enum bpf_prog_status {
901-
BPF_PROG_STATUS_DEV_BOUND = (1 << 0),
902-
};
903-
904900
struct bpf_prog_info {
905901
__u32 type;
906902
__u32 id;
@@ -914,8 +910,6 @@ struct bpf_prog_info {
914910
__u32 nr_map_ids;
915911
__aligned_u64 map_ids;
916912
char name[BPF_OBJ_NAME_LEN];
917-
__u32 ifindex;
918-
__u32 status;
919913
} __attribute__((aligned(8)));
920914

921915
struct bpf_map_info {

0 commit comments

Comments
 (0)