Skip to contentSkip to conversation

Misc cls_bpf/act_bpf improvements

PATCH In review Daniel Borkmann 2 participants 4 messages Sep 12, 2016 – Sep 15, 2016

Subsystems (5)
lore
Share & tools
lore

Daniel Borkmann

author maintainer
#0 lore raw

cover Misc cls_bpf/act_bpf improvements

Two minor improvements to {cls,act}_bpf. For details please see individual patches.

Thanks!

Daniel Borkmann (2):
bpf: drop unnecessary test in cls_bpf_classify and tcf_bpf
bpf: use skb_at_tc_ingress helper in tcf_bpf

 net/sched/act_bpf.c | 5 +----
 net/sched/cls_bpf.c | 3 ---
 2 files changed, 1 insertion(+), 7 deletions(-)
Signature · 1 line
1.9.3
author maintainer
#1 lore raw

PATCH 1/2 bpf: drop unnecessary test in cls_bpf_classify and tcf_bpf

The skb_mac_header_was_set() test in cls_bpf's and act_bpf's fast-path is actually unnecessary and can be removed altogether. This was added by commit a166151cbe33 ("bpf: fix bpf helpers to use skb->mac_header relative offsets"), which was later on improved by 3431205e0397 ("bpf: make programs see skb->data == L2 for ingress and egress"). We're always guaranteed to have valid mac header at the time we invoke cls_bpf_classify() or tcf_bpf().

Reason is that since 6d1ccff62780 ("net: reset mac header in dev_start_xmit()") we do skb_reset_mac_header() in __dev_queue_xmit() before we could call into sch_handle_egress() or any subsequent enqueue. sch_handle_ingress() always sees a valid mac header as well (things like skb_reset_mac_len() would badly fail otherwise). Thus, drop the unnecessary test in classifier and action case.

---
 net/sched/act_bpf.c | 3 ---
 net/sched/cls_bpf.c | 3 ---
 2 files changed, 6 deletions(-)
net/sched/act_bpf.c +0 -3
index bfa8707..78400de 100644
@@ -44,9 +44,6 @@ static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
 	int action, filter_res;
 	bool at_ingress = G_TC_AT(skb->tc_verd) & AT_INGRESS;
 
-	if (unlikely(!skb_mac_header_was_set(skb)))
-		return TC_ACT_UNSPEC;
-
 	tcf_lastuse_update(&prog->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
 
net/sched/cls_bpf.c +0 -3
index 4742f41..1d92d4d 100644
@@ -83,9 +83,6 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 	struct cls_bpf_prog *prog;
 	int ret = -1;
 
-	if (unlikely(!skb_mac_header_was_set(skb)))
-		return -1;
-
 	/* Needed here for accessing maps. */
 	rcu_read_lock();
 	list_for_each_entry_rcu(prog, &head->plist, link) {
Signature · 1 line
1.9.3
author maintainer
#2 lore raw

PATCH 2/2 bpf: use skb_at_tc_ingress helper in tcf_bpf

In reply to Daniel Borkmann

We have a small skb_at_tc_ingress() helper for testing for ingress, so make use of it. cls_bpf already uses it and so should act_bpf.

---
 net/sched/act_bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/sched/act_bpf.c +1 -1
index 78400de..1d39600 100644
@@ -39,10 +39,10 @@ static struct tc_action_ops act_bpf_ops;
 static int tcf_bpf(struct sk_buff *skb, const struct tc_action *act,
 		   struct tcf_result *res)
 {
+	bool at_ingress = skb_at_tc_ingress(skb);
 	struct tcf_bpf *prog = to_bpf(act);
 	struct bpf_prog *filter;
 	int action, filter_res;
-	bool at_ingress = G_TC_AT(skb->tc_verd) & AT_INGRESS;
 
 	tcf_lastuse_update(&prog->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(prog->common.cpu_bstats), skb);
Signature · 1 line
1.9.3