[PATCH net-next v4 0/4] TC: refactor act_mirred packets re-injection

[PATCH net-next v4 0/4] TC: refactor act_mirred packets re-injection

[Paolo Abeni]

This series is aimed at improving the act_mirred redirect performances.
Such action is used by OVS to represent TC S/W flows, and it's current largest
bottle-neck is the need for a skb_clone() for each packet.

The first 2 patches introduce some cleanup and safeguards to allow extending 
tca_result - we will use it to store RCU protected redirect information - and
introduce a clear separation between user-space accessible tcfa_action
values and internal values accessible only by the kernel.
Then a new tcfa_action value is introduced: TC_ACT_REINJECT, similar to
TC_ACT_REDIRECT, but preserving the mirred semantic. Such value is not
accessible from user-space.
The last patch exploits the newly introduced infrastructure in the act_mirred
action, to avoid a skb_clone, when possible.

Overall this the above gives a ~10% performance improvement in forwarding tput,
when using the TC S/W datapath.

v1 -> v2:
 - preserve the rcu lock in act_bpf
 - add and use a new action value to reinject the packets, preserving the mirred
   semantic

v2 -> v3:
 - renamed to new action as TC_ACT_REINJECT
 - TC_ACT_REINJECT is not exposed to user-space

v3 -> v4:
 - dropped the TC_ACT_REDIRECT patch
 - report failure via extack, too
 - rename the new action as TC_ACT_REINSERT
 - skip clone only if the control action don't touch tcf_result

Paolo Abeni (4):
  net/sched: user-space can't set unknown tcfa_action values
  tc/act: remove unneeded RCU lock in action callback
  net/tc: introduce TC_ACT_REINSERT.
  act_mirred: use TC_ACT_REINSERT when possible

 include/net/act_api.h        |  2 +-
 include/net/pkt_cls.h        |  3 ++
 include/net/sch_generic.h    | 21 +++++++++++++
 include/uapi/linux/pkt_cls.h |  6 ++--
 net/core/dev.c               |  6 +++-
 net/sched/act_api.c          | 17 +++++++++++
 net/sched/act_csum.c         | 12 ++------
 net/sched/act_ife.c          |  5 +--
 net/sched/act_mirred.c       | 59 +++++++++++++++++++++++++++---------
 net/sched/act_sample.c       |  4 +--
 net/sched/act_skbedit.c      | 10 ++----
 net/sched/act_skbmod.c       | 21 ++++++-------
 net/sched/act_tunnel_key.c   |  6 +---
 net/sched/act_vlan.c         | 19 +++++-------
 14 files changed, 121 insertions(+), 70 deletions(-)

-- 
2.17.1

[PATCH net-next v4 1/4] net/sched: user-space can't set unknown tcfa_action values

[Paolo Abeni]

Currently, when initializing an action, the user-space can specify
and use arbitrary values for the tcfa_action field. If the value
is unknown by the kernel, is implicitly threaded as TC_ACT_UNSPEC.

This change explicitly checks for unknown values at action creation
time, and explicitly convert them to TC_ACT_UNSPEC. No functional
changes are introduced, but this will allow introducing tcfa_action
values not exposed to user-space in a later patch.

Note: we can't use the above to hide TC_ACT_REDIRECT from user-space,
as the latter is already part of uAPI.

v3 -> v4:
 - use an helper to check for action validity (JiriP)
 - emit an extack for invalid actions (JiriP)

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/uapi/linux/pkt_cls.h |  6 ++++--
 net/sched/act_api.c          | 17 +++++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index b4512254036b..48e5b5d49a34 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -45,6 +45,7 @@ enum {
 				   * the skb and act like everything
 				   * is alright.
 				   */
+#define TC_ACT_VALUE_MAX	TC_ACT_TRAP
 
 /* There is a special kind of actions called "extended actions",
  * which need a value parameter. These have a local opcode located in
@@ -55,11 +56,12 @@ enum {
 #define __TC_ACT_EXT_SHIFT 28
 #define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
 #define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
-#define TC_ACT_EXT_CMP(combined, opcode) \
-	(((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
+#define TC_ACT_EXT_OPCODE(combined) ((combined) & (~TC_ACT_EXT_VAL_MASK))
+#define TC_ACT_EXT_CMP(combined, opcode) (TC_ACT_EXT_OPCODE(combined) == opcode)
 
 #define TC_ACT_JUMP __TC_ACT_EXT(1)
 #define TC_ACT_GOTO_CHAIN __TC_ACT_EXT(2)
+#define TC_ACT_EXT_OPCODE_MAX	TC_ACT_GOTO_CHAIN
 
 /* Action type identifiers*/
 enum {
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 148a89ab789b..bdccad583daf 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -786,6 +786,15 @@ static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
 	return c;
 }
 
+static bool tcf_action_valid(int action)
+{
+	int opcode = TC_ACT_EXT_OPCODE(action);
+
+	if (!opcode)
+		return action <= TC_ACT_VALUE_MAX;
+	return opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC;
+}
+
 struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 				    struct nlattr *nla, struct nlattr *est,
 				    char *name, int ovr, int bind,
@@ -895,6 +904,14 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 		}
 	}
 
+	if (!tcf_action_valid(a->tcfa_action)) {
+		net_warn_ratelimited("invalid %d action value, using "
+				     "TC_ACT_UNSPEC instead", a->tcfa_action);
+		NL_SET_ERR_MSG(extack, "invalid action value, using "
+			       "TC_ACT_UNSPEC instead");
+		a->tcfa_action = TC_ACT_UNSPEC;
+	}
+
 	return a;
 
 err_mod:
-- 
2.17.1

Re: [PATCH net-next v4 1/4] net/sched: user-space can't set unknown tcfa_action values

[Marcelo Ricardo Leitner]

Hi,

On Thu, Jul 26, 2018 at 04:34:57PM +0200, Paolo Abeni wrote:
...
> @@ -895,6 +904,14 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>  		}
>  	}
>  
> +	if (!tcf_action_valid(a->tcfa_action)) {
> +		net_warn_ratelimited("invalid %d action value, using "
> +				     "TC_ACT_UNSPEC instead", a->tcfa_action);

Now that it is reporting the error via extack, do we really need this
warn net_warn?
extack will be shown as a warning by iproute2 even if the command
succeeds.

> +		NL_SET_ERR_MSG(extack, "invalid action value, using "
> +			       "TC_ACT_UNSPEC instead");

Quoted strings shouldn't be broken down into multiple lines..

> +		a->tcfa_action = TC_ACT_UNSPEC;
> +	}
> +
>  	return a;
>  
>  err_mod:
> -- 
> 2.17.1
> 

Re: [PATCH net-next v4 1/4] net/sched: user-space can't set unknown tcfa_action values

[Paolo Abeni]

On Thu, 2018-07-26 at 21:28 -0300, Marcelo Ricardo Leitner wrote:
> Hi,
> 
> On Thu, Jul 26, 2018 at 04:34:57PM +0200, Paolo Abeni wrote:
> ...
> > @@ -895,6 +904,14 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> >  		}
> >  	}
> >  
> > +	if (!tcf_action_valid(a->tcfa_action)) {
> > +		net_warn_ratelimited("invalid %d action value, using "
> > +				     "TC_ACT_UNSPEC instead", a->tcfa_action);
> 
> Now that it is reporting the error via extack, do we really need this
> warn net_warn?
> extack will be shown as a warning by iproute2 even if the command
> succeeds.

That was requested by Jiri (modulo misinterpretation on my side).
My understanding is that the extact will warn the whoever tryed to push
the bad configuration, while the net_warn is targeting the hosts
administrator.

Jiri, do you have strong opinion on this or did I misinterpret your
wording/ can I drop the net_warn?

Thanks!

> > +		NL_SET_ERR_MSG(extack, "invalid action value, using "
> > +			       "TC_ACT_UNSPEC instead");
> 
> Quoted strings shouldn't be broken down into multiple lines..

Thanks, 

will fix in v5 :(

Cheers,

Paolo

[PATCH net-next v4 2/4] tc/act: remove unneeded RCU lock in action callback

[Paolo Abeni]

Each lockless action currently does its own RCU locking in ->act().
This allows using plain RCU accessor, even if the context
is really RCU BH.

This change drops the per action RCU lock, replace the accessors
with the _bh variant, cleans up a bit the surrounding code and
documents the RCU status in the relevant header.
No functional nor performance change is intended.

The goal of this patch is clarifying that the RCU critical section
used by the tc actions extends up to the classifier's caller.

v1 -> v2:
 - preserve rcu lock in act_bpf: it's needed by eBPF helpers,
   as pointed out by Daniel

v3 -> v4:
 - fixed some typos in the commit message (JiriP)

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/act_api.h      |  2 +-
 include/net/sch_generic.h  |  2 ++
 net/sched/act_csum.c       | 12 +++---------
 net/sched/act_ife.c        |  5 +----
 net/sched/act_mirred.c     |  4 +---
 net/sched/act_sample.c     |  4 +---
 net/sched/act_skbedit.c    | 10 +++-------
 net/sched/act_skbmod.c     | 21 +++++++++------------
 net/sched/act_tunnel_key.c |  6 +-----
 net/sched/act_vlan.c       | 19 +++++++------------
 10 files changed, 29 insertions(+), 56 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 683ce41053d9..8c9bc02d05e1 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -85,7 +85,7 @@ struct tc_action_ops {
 	size_t	size;
 	struct module		*owner;
 	int     (*act)(struct sk_buff *, const struct tc_action *,
-		       struct tcf_result *);
+		       struct tcf_result *); /* called under RCU BH lock*/
 	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);
 	void	(*cleanup)(struct tc_action *);
 	int     (*lookup)(struct net *net, struct tc_action **a, u32 index,
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 085c509c8674..c9af9ce33055 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -285,6 +285,8 @@ struct tcf_proto {
 	/* Fast access part */
 	struct tcf_proto __rcu	*next;
 	void __rcu		*root;
+
+	/* called under RCU BH lock*/
 	int			(*classify)(struct sk_buff *,
 					    const struct tcf_proto *,
 					    struct tcf_result *);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 4e8c383f379e..648a3a35b720 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -561,15 +561,14 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a,
 	u32 update_flags;
 	int action;
 
-	rcu_read_lock();
-	params = rcu_dereference(p->params);
+	params = rcu_dereference_bh(p->params);
 
 	tcf_lastuse_update(&p->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(p->common.cpu_bstats), skb);
 
 	action = READ_ONCE(p->tcf_action);
 	if (unlikely(action == TC_ACT_SHOT))
-		goto drop_stats;
+		goto drop;
 
 	update_flags = params->update_flags;
 	switch (tc_skb_protocol(skb)) {
@@ -583,16 +582,11 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a,
 		break;
 	}
 
-unlock:
-	rcu_read_unlock();
 	return action;
 
 drop:
-	action = TC_ACT_SHOT;
-
-drop_stats:
 	qstats_drop_inc(this_cpu_ptr(p->common.cpu_qstats));
-	goto unlock;
+	return TC_ACT_SHOT;
 }
 
 static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 3d6e265758c0..df4060e32d43 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -820,14 +820,11 @@ static int tcf_ife_act(struct sk_buff *skb, const struct tc_action *a,
 	struct tcf_ife_params *p;
 	int ret;
 
-	rcu_read_lock();
-	p = rcu_dereference(ife->params);
+	p = rcu_dereference_bh(ife->params);
 	if (p->flags & IFE_ENCODE) {
 		ret = tcf_ife_encode(skb, a, res, p);
-		rcu_read_unlock();
 		return ret;
 	}
-	rcu_read_unlock();
 
 	return tcf_ife_decode(skb, a, res);
 }
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 6afd89a36c69..eeb335f03102 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -181,11 +181,10 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 	tcf_lastuse_update(&m->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
 
-	rcu_read_lock();
 	m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
 	m_eaction = READ_ONCE(m->tcfm_eaction);
 	retval = READ_ONCE(m->tcf_action);
-	dev = rcu_dereference(m->tcfm_dev);
+	dev = rcu_dereference_bh(m->tcfm_dev);
 	if (unlikely(!dev)) {
 		pr_notice_once("tc mirred: target device is gone\n");
 		goto out;
@@ -236,7 +235,6 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		if (tcf_mirred_is_act_redirect(m_eaction))
 			retval = TC_ACT_SHOT;
 	}
-	rcu_read_unlock();
 
 	return retval;
 }
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 3079e7be5bde..2608ccc83e5e 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -140,8 +140,7 @@ static int tcf_sample_act(struct sk_buff *skb, const struct tc_action *a,
 	bstats_cpu_update(this_cpu_ptr(s->common.cpu_bstats), skb);
 	retval = READ_ONCE(s->tcf_action);
 
-	rcu_read_lock();
-	psample_group = rcu_dereference(s->psample_group);
+	psample_group = rcu_dereference_bh(s->psample_group);
 
 	/* randomly sample packets according to rate */
 	if (psample_group && (prandom_u32() % s->rate == 0)) {
@@ -165,7 +164,6 @@ static int tcf_sample_act(struct sk_buff *skb, const struct tc_action *a,
 			skb_pull(skb, skb->mac_len);
 	}
 
-	rcu_read_unlock();
 	return retval;
 }
 
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index da56e6938c9e..a6db47ebec11 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -43,8 +43,7 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 	tcf_lastuse_update(&d->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(d->common.cpu_bstats), skb);
 
-	rcu_read_lock();
-	params = rcu_dereference(d->params);
+	params = rcu_dereference_bh(d->params);
 	action = READ_ONCE(d->tcf_action);
 
 	if (params->flags & SKBEDIT_F_PRIORITY)
@@ -77,14 +76,11 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
 	}
 	if (params->flags & SKBEDIT_F_PTYPE)
 		skb->pkt_type = params->ptype;
-
-unlock:
-	rcu_read_unlock();
 	return action;
+
 err:
 	qstats_drop_inc(this_cpu_ptr(d->common.cpu_qstats));
-	action = TC_ACT_SHOT;
-	goto unlock;
+	return TC_ACT_SHOT;
 }
 
 static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = {
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index cdc6bacfb190..c437c6d51a71 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -41,20 +41,14 @@ static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
 	 * then MAX_EDIT_LEN needs to change appropriately
 	*/
 	err = skb_ensure_writable(skb, MAX_EDIT_LEN);
-	if (unlikely(err)) { /* best policy is to drop on the floor */
-		qstats_overlimit_inc(this_cpu_ptr(d->common.cpu_qstats));
-		return TC_ACT_SHOT;
-	}
+	if (unlikely(err)) /* best policy is to drop on the floor */
+		goto drop;
 
-	rcu_read_lock();
 	action = READ_ONCE(d->tcf_action);
-	if (unlikely(action == TC_ACT_SHOT)) {
-		qstats_overlimit_inc(this_cpu_ptr(d->common.cpu_qstats));
-		rcu_read_unlock();
-		return action;
-	}
+	if (unlikely(action == TC_ACT_SHOT))
+		goto drop;
 
-	p = rcu_dereference(d->skbmod_p);
+	p = rcu_dereference_bh(d->skbmod_p);
 	flags = p->flags;
 	if (flags & SKBMOD_F_DMAC)
 		ether_addr_copy(eth_hdr(skb)->h_dest, p->eth_dst);
@@ -62,7 +56,6 @@ static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
 		ether_addr_copy(eth_hdr(skb)->h_source, p->eth_src);
 	if (flags & SKBMOD_F_ETYPE)
 		eth_hdr(skb)->h_proto = p->eth_type;
-	rcu_read_unlock();
 
 	if (flags & SKBMOD_F_SWAPMAC) {
 		u16 tmpaddr[ETH_ALEN / 2]; /* ether_addr_copy() requirement */
@@ -73,6 +66,10 @@ static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
 	}
 
 	return action;
+
+drop:
+	qstats_overlimit_inc(this_cpu_ptr(d->common.cpu_qstats));
+	return TC_ACT_SHOT;
 }
 
 static const struct nla_policy skbmod_policy[TCA_SKBMOD_MAX + 1] = {
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index f811850fd1d0..d42d9e112789 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -31,9 +31,7 @@ static int tunnel_key_act(struct sk_buff *skb, const struct tc_action *a,
 	struct tcf_tunnel_key_params *params;
 	int action;
 
-	rcu_read_lock();
-
-	params = rcu_dereference(t->params);
+	params = rcu_dereference_bh(t->params);
 
 	tcf_lastuse_update(&t->tcf_tm);
 	bstats_cpu_update(this_cpu_ptr(t->common.cpu_bstats), skb);
@@ -53,8 +51,6 @@ static int tunnel_key_act(struct sk_buff *skb, const struct tc_action *a,
 		break;
 	}
 
-	rcu_read_unlock();
-
 	return action;
 }
 
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index ad37f308175a..15a0ee214c9c 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -40,11 +40,9 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
 	if (skb_at_tc_ingress(skb))
 		skb_push_rcsum(skb, skb->mac_len);
 
-	rcu_read_lock();
-
 	action = READ_ONCE(v->tcf_action);
 
-	p = rcu_dereference(v->vlan_p);
+	p = rcu_dereference_bh(v->vlan_p);
 
 	switch (p->tcfv_action) {
 	case TCA_VLAN_ACT_POP:
@@ -61,7 +59,7 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
 	case TCA_VLAN_ACT_MODIFY:
 		/* No-op if no vlan tag (either hw-accel or in-payload) */
 		if (!skb_vlan_tagged(skb))
-			goto unlock;
+			goto out;
 		/* extract existing tag (and guarantee no hw-accel tag) */
 		if (skb_vlan_tag_present(skb)) {
 			tci = skb_vlan_tag_get(skb);
@@ -86,18 +84,15 @@ static int tcf_vlan(struct sk_buff *skb, const struct tc_action *a,
 		BUG();
 	}
 
-	goto unlock;
-
-drop:
-	action = TC_ACT_SHOT;
-	qstats_drop_inc(this_cpu_ptr(v->common.cpu_qstats));
-
-unlock:
-	rcu_read_unlock();
+out:
 	if (skb_at_tc_ingress(skb))
 		skb_pull_rcsum(skb, skb->mac_len);
 
 	return action;
+
+drop:
+	qstats_drop_inc(this_cpu_ptr(v->common.cpu_qstats));
+	return TC_ACT_SHOT;
 }
 
 static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = {
-- 
2.17.1

[PATCH net-next v4 3/4] net/tc: introduce TC_ACT_REINSERT.

[Paolo Abeni]

This is similar TC_ACT_REDIRECT, but with a slightly different
semantic:
- on ingress the mirred skbs are passed to the target device
network stack without any additional check not scrubbing.
- the rcu-protected stats provided via the tcf_result struct
  are updated on error conditions.

This new tcfa_action value is not exposed to the user-space
and can be used only internally by clsact.

v1 -> v2: do not touch TC_ACT_REDIRECT code path, introduce
 a new action type instead

v2 -> v3:
 - rename the new action value TC_ACT_REINJECT, update the
   helper accordingly
 - take care of uncloned reinjected packets in XDP generic
   hook

v3 -> v4:
 - renamed again the new action value (JiriP)

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: this patch still touch only overlimits, even there is some 
agreement to touch (also) drops on reinsert/mirred failure, but
such change is independent to this series
---
 include/net/pkt_cls.h     |  3 +++
 include/net/sch_generic.h | 19 +++++++++++++++++++
 net/core/dev.c            |  6 +++++-
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index a3101582f642..d64915273358 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -7,6 +7,9 @@
 #include <net/sch_generic.h>
 #include <net/act_api.h>
 
+/* TC action not accessible from user space */
+#define TC_ACT_REINSERT		(TC_ACT_VALUE_MAX + 1)
+
 /* Basic packet classifier frontend definitions. */
 
 struct tcf_walker {
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index c9af9ce33055..70270d9c1bb1 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -235,6 +235,12 @@ struct tcf_result {
 			u32		classid;
 		};
 		const struct tcf_proto *goto_tp;
+
+		/* used by the TC_ACT_REINSERT action */
+		struct {
+			bool		ingress;
+			struct gnet_stats_queue *qstats;
+		};
 	};
 };
 
@@ -1107,4 +1113,17 @@ void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
 void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
 			  struct mini_Qdisc __rcu **p_miniq);
 
+static inline void skb_tc_reinsert(struct sk_buff *skb, struct tcf_result *res)
+{
+	struct gnet_stats_queue *stats = res->qstats;
+	int ret;
+
+	if (res->ingress)
+		ret = netif_receive_skb(skb);
+	else
+		ret = dev_queue_xmit(skb);
+	if (ret && stats)
+		qstats_overlimit_inc(res->qstats);
+}
+
 #endif
diff --git a/net/core/dev.c b/net/core/dev.c
index 87c42c8249ae..d5faa6449fb2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4252,7 +4252,7 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,
 	/* Reinjected packets coming from act_mirred or similar should
 	 * not get XDP generic processing.
 	 */
-	if (skb_cloned(skb))
+	if (skb_cloned(skb) || skb->tc_redirected)
 		return XDP_PASS;
 
 	/* XDP packets must be linear and must have sufficient headroom
@@ -4602,6 +4602,10 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
 		__skb_push(skb, skb->mac_len);
 		skb_do_redirect(skb);
 		return NULL;
+	case TC_ACT_REINSERT:
+		/* this does not scrub the packet, and updates stats on error */
+		skb_tc_reinsert(skb, &cl_res);
+		return NULL;
 	default:
 		break;
 	}
-- 
2.17.1

Re: [PATCH net-next v4 3/4] net/tc: introduce TC_ACT_REINSERT.

[Cong Wang]

On Thu, Jul 26, 2018 at 7:35 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This is similar TC_ACT_REDIRECT, but with a slightly different
> semantic:
> - on ingress the mirred skbs are passed to the target device
> network stack without any additional check not scrubbing.
> - the rcu-protected stats provided via the tcf_result struct
>   are updated on error conditions.
>
> This new tcfa_action value is not exposed to the user-space
> and can be used only internally by clsact.
>
> v1 -> v2: do not touch TC_ACT_REDIRECT code path, introduce
>  a new action type instead
>
> v2 -> v3:
>  - rename the new action value TC_ACT_REINJECT, update the
>    helper accordingly
>  - take care of uncloned reinjected packets in XDP generic
>    hook
>
> v3 -> v4:
>  - renamed again the new action value (JiriP)
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>


Acked-by: Cong Wang <xiyou.wangcong@gmail.com>


> ---
> Note: this patch still touch only overlimits, even there is some
> agreement to touch (also) drops on reinsert/mirred failure, but
> such change is independent to this series

Totally agree.

Thanks!

Re: [PATCH net-next v4 3/4] net/tc: introduce TC_ACT_REINSERT.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3941 bytes --]

Hi Paolo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/TC-refactor-act_mirred-packets-re-injection/20180729-102154
config: i386-randconfig-n0-201829 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   net//core/dev.c: In function 'netif_receive_generic_xdp':
>> net//core/dev.c:4255:28: error: 'struct sk_buff' has no member named 'tc_redirected'
     if (skb_cloned(skb) || skb->tc_redirected)
                               ^~

vim +4255 net//core/dev.c

  4241	
  4242	static u32 netif_receive_generic_xdp(struct sk_buff *skb,
  4243					     struct xdp_buff *xdp,
  4244					     struct bpf_prog *xdp_prog)
  4245	{
  4246		struct netdev_rx_queue *rxqueue;
  4247		void *orig_data, *orig_data_end;
  4248		u32 metalen, act = XDP_DROP;
  4249		int hlen, off;
  4250		u32 mac_len;
  4251	
  4252		/* Reinjected packets coming from act_mirred or similar should
  4253		 * not get XDP generic processing.
  4254		 */
> 4255		if (skb_cloned(skb) || skb->tc_redirected)
  4256			return XDP_PASS;
  4257	
  4258		/* XDP packets must be linear and must have sufficient headroom
  4259		 * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
  4260		 * native XDP provides, thus we need to do it here as well.
  4261		 */
  4262		if (skb_is_nonlinear(skb) ||
  4263		    skb_headroom(skb) < XDP_PACKET_HEADROOM) {
  4264			int hroom = XDP_PACKET_HEADROOM - skb_headroom(skb);
  4265			int troom = skb->tail + skb->data_len - skb->end;
  4266	
  4267			/* In case we have to go down the path and also linearize,
  4268			 * then lets do the pskb_expand_head() work just once here.
  4269			 */
  4270			if (pskb_expand_head(skb,
  4271					     hroom > 0 ? ALIGN(hroom, NET_SKB_PAD) : 0,
  4272					     troom > 0 ? troom + 128 : 0, GFP_ATOMIC))
  4273				goto do_drop;
  4274			if (skb_linearize(skb))
  4275				goto do_drop;
  4276		}
  4277	
  4278		/* The XDP program wants to see the packet starting at the MAC
  4279		 * header.
  4280		 */
  4281		mac_len = skb->data - skb_mac_header(skb);
  4282		hlen = skb_headlen(skb) + mac_len;
  4283		xdp->data = skb->data - mac_len;
  4284		xdp->data_meta = xdp->data;
  4285		xdp->data_end = xdp->data + hlen;
  4286		xdp->data_hard_start = skb->data - skb_headroom(skb);
  4287		orig_data_end = xdp->data_end;
  4288		orig_data = xdp->data;
  4289	
  4290		rxqueue = netif_get_rxqueue(skb);
  4291		xdp->rxq = &rxqueue->xdp_rxq;
  4292	
  4293		act = bpf_prog_run_xdp(xdp_prog, xdp);
  4294	
  4295		off = xdp->data - orig_data;
  4296		if (off > 0)
  4297			__skb_pull(skb, off);
  4298		else if (off < 0)
  4299			__skb_push(skb, -off);
  4300		skb->mac_header += off;
  4301	
  4302		/* check if bpf_xdp_adjust_tail was used. it can only "shrink"
  4303		 * pckt.
  4304		 */
  4305		off = orig_data_end - xdp->data_end;
  4306		if (off != 0) {
  4307			skb_set_tail_pointer(skb, xdp->data_end - xdp->data);
  4308			skb->len -= off;
  4309	
  4310		}
  4311	
  4312		switch (act) {
  4313		case XDP_REDIRECT:
  4314		case XDP_TX:
  4315			__skb_push(skb, mac_len);
  4316			break;
  4317		case XDP_PASS:
  4318			metalen = xdp->data - xdp->data_meta;
  4319			if (metalen)
  4320				skb_metadata_set(skb, metalen);
  4321			break;
  4322		default:
  4323			bpf_warn_invalid_xdp_action(act);
  4324			/* fall through */
  4325		case XDP_ABORTED:
  4326			trace_xdp_exception(skb->dev, xdp_prog, act);
  4327			/* fall through */
  4328		case XDP_DROP:
  4329		do_drop:
  4330			kfree_skb(skb);
  4331			break;
  4332		}
  4333	
  4334		return act;
  4335	}
  4336	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26257 bytes --]

Re: [PATCH net-next v4 3/4] net/tc: introduce TC_ACT_REINSERT.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 5330 bytes --]

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/TC-refactor-act_mirred-packets-re-injection/20180729-102154
config: x86_64-randconfig-u0-07291027 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/current.h:5:0,
                    from include/linux/sched.h:12,
                    from include/linux/uaccess.h:5,
                    from net/core/dev.c:75:
   net/core/dev.c: In function 'netif_receive_generic_xdp':
   net/core/dev.c:4255:28: error: 'struct sk_buff' has no member named 'tc_redirected'
     if (skb_cloned(skb) || skb->tc_redirected)
                               ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^
>> net/core/dev.c:4255:2: note: in expansion of macro 'if'
     if (skb_cloned(skb) || skb->tc_redirected)
     ^
   net/core/dev.c:4255:28: error: 'struct sk_buff' has no member named 'tc_redirected'
     if (skb_cloned(skb) || skb->tc_redirected)
                               ^
   include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^
>> net/core/dev.c:4255:2: note: in expansion of macro 'if'
     if (skb_cloned(skb) || skb->tc_redirected)
     ^
   net/core/dev.c:4255:28: error: 'struct sk_buff' has no member named 'tc_redirected'
     if (skb_cloned(skb) || skb->tc_redirected)
                               ^
   include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^
>> net/core/dev.c:4255:2: note: in expansion of macro 'if'
     if (skb_cloned(skb) || skb->tc_redirected)
     ^

vim +/if +4255 net/core/dev.c

  4241	
  4242	static u32 netif_receive_generic_xdp(struct sk_buff *skb,
  4243					     struct xdp_buff *xdp,
  4244					     struct bpf_prog *xdp_prog)
  4245	{
  4246		struct netdev_rx_queue *rxqueue;
  4247		void *orig_data, *orig_data_end;
  4248		u32 metalen, act = XDP_DROP;
  4249		int hlen, off;
  4250		u32 mac_len;
  4251	
  4252		/* Reinjected packets coming from act_mirred or similar should
  4253		 * not get XDP generic processing.
  4254		 */
> 4255		if (skb_cloned(skb) || skb->tc_redirected)
  4256			return XDP_PASS;
  4257	
  4258		/* XDP packets must be linear and must have sufficient headroom
  4259		 * of XDP_PACKET_HEADROOM bytes. This is the guarantee that also
  4260		 * native XDP provides, thus we need to do it here as well.
  4261		 */
  4262		if (skb_is_nonlinear(skb) ||
  4263		    skb_headroom(skb) < XDP_PACKET_HEADROOM) {
  4264			int hroom = XDP_PACKET_HEADROOM - skb_headroom(skb);
  4265			int troom = skb->tail + skb->data_len - skb->end;
  4266	
  4267			/* In case we have to go down the path and also linearize,
  4268			 * then lets do the pskb_expand_head() work just once here.
  4269			 */
  4270			if (pskb_expand_head(skb,
  4271					     hroom > 0 ? ALIGN(hroom, NET_SKB_PAD) : 0,
  4272					     troom > 0 ? troom + 128 : 0, GFP_ATOMIC))
  4273				goto do_drop;
  4274			if (skb_linearize(skb))
  4275				goto do_drop;
  4276		}
  4277	
  4278		/* The XDP program wants to see the packet starting at the MAC
  4279		 * header.
  4280		 */
  4281		mac_len = skb->data - skb_mac_header(skb);
  4282		hlen = skb_headlen(skb) + mac_len;
  4283		xdp->data = skb->data - mac_len;
  4284		xdp->data_meta = xdp->data;
  4285		xdp->data_end = xdp->data + hlen;
  4286		xdp->data_hard_start = skb->data - skb_headroom(skb);
  4287		orig_data_end = xdp->data_end;
  4288		orig_data = xdp->data;
  4289	
  4290		rxqueue = netif_get_rxqueue(skb);
  4291		xdp->rxq = &rxqueue->xdp_rxq;
  4292	
  4293		act = bpf_prog_run_xdp(xdp_prog, xdp);
  4294	
  4295		off = xdp->data - orig_data;
  4296		if (off > 0)
  4297			__skb_pull(skb, off);
  4298		else if (off < 0)
  4299			__skb_push(skb, -off);
  4300		skb->mac_header += off;
  4301	
  4302		/* check if bpf_xdp_adjust_tail was used. it can only "shrink"
  4303		 * pckt.
  4304		 */
  4305		off = orig_data_end - xdp->data_end;
  4306		if (off != 0) {
  4307			skb_set_tail_pointer(skb, xdp->data_end - xdp->data);
  4308			skb->len -= off;
  4309	
  4310		}
  4311	
  4312		switch (act) {
  4313		case XDP_REDIRECT:
  4314		case XDP_TX:
  4315			__skb_push(skb, mac_len);
  4316			break;
  4317		case XDP_PASS:
  4318			metalen = xdp->data - xdp->data_meta;
  4319			if (metalen)
  4320				skb_metadata_set(skb, metalen);
  4321			break;
  4322		default:
  4323			bpf_warn_invalid_xdp_action(act);
  4324			/* fall through */
  4325		case XDP_ABORTED:
  4326			trace_xdp_exception(skb->dev, xdp_prog, act);
  4327			/* fall through */
  4328		case XDP_DROP:
  4329		do_drop:
  4330			kfree_skb(skb);
  4331			break;
  4332		}
  4333	
  4334		return act;
  4335	}
  4336	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34296 bytes --]

[PATCH net-next v4 4/4] act_mirred: use TC_ACT_REINSERT when possible

[Paolo Abeni]

When mirred is invoked from the ingress path, and it wants to redirect
the processed packet, it can now use the TC_ACT_REINSERT action,
filling the tcf_result accordingly, and avoiding a per packet
skb_clone().

Overall this gives a ~10% improvement in forwarding performance for the
TC S/W data path and TC S/W performances are now comparable to the
kernel openvswitch datapath.

v1 -> v2: use ACT_MIRRED instead of ACT_REDIRECT
v2 -> v3: updated after action rename, fixed typo into the commit
	message
v3 -> v4: updated again after action rename, added more comments to
	the code (JiriP), skip the optimization if the control action
	need to touch the tcf_result (Paolo)

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/sched/act_mirred.c | 55 +++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index eeb335f03102..d5f1a8102b7b 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -25,6 +25,7 @@
 #include <net/net_namespace.h>
 #include <net/netlink.h>
 #include <net/pkt_sched.h>
+#include <net/pkt_cls.h>
 #include <linux/tc_act/tc_mirred.h>
 #include <net/tc_act/tc_mirred.h>
 
@@ -49,6 +50,18 @@ static bool tcf_mirred_act_wants_ingress(int action)
 	}
 }
 
+static bool tcf_mirred_can_reinsert(int action)
+{
+	switch (action) {
+	case TC_ACT_SHOT:
+	case TC_ACT_STOLEN:
+	case TC_ACT_QUEUED:
+	case TC_ACT_TRAP:
+		return true;
+	}
+	return false;
+}
+
 static void tcf_mirred_release(struct tc_action *a)
 {
 	struct tcf_mirred *m = to_mirred(a);
@@ -171,10 +184,13 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		      struct tcf_result *res)
 {
 	struct tcf_mirred *m = to_mirred(a);
+	struct sk_buff *skb2 = skb;
 	bool m_mac_header_xmit;
 	struct net_device *dev;
-	struct sk_buff *skb2;
 	int retval, err = 0;
+	bool use_reinsert;
+	bool want_ingress;
+	bool is_redirect;
 	int m_eaction;
 	int mac_len;
 
@@ -196,16 +212,25 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		goto out;
 	}
 
-	skb2 = skb_clone(skb, GFP_ATOMIC);
-	if (!skb2)
-		goto out;
+	/* we could easily avoid the clone only if called by ingress and clsact;
+	 * since we can't easily detect the clsact caller, skip clone only for
+	 * ingress - that covers the TC S/W datapath.
+	 */
+	is_redirect = tcf_mirred_is_act_redirect(m_eaction);
+	use_reinsert = skb_at_tc_ingress(skb) && is_redirect &&
+		       tcf_mirred_can_reinsert(retval);
+	if (!use_reinsert) {
+		skb2 = skb_clone(skb, GFP_ATOMIC);
+		if (!skb2)
+			goto out;
+	}
 
 	/* If action's target direction differs than filter's direction,
 	 * and devices expect a mac header on xmit, then mac push/pull is
 	 * needed.
 	 */
-	if (skb_at_tc_ingress(skb) != tcf_mirred_act_wants_ingress(m_eaction) &&
-	    m_mac_header_xmit) {
+	want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
+	if (skb_at_tc_ingress(skb) != want_ingress && m_mac_header_xmit) {
 		if (!skb_at_tc_ingress(skb)) {
 			/* caught at egress, act ingress: pull mac */
 			mac_len = skb_network_header(skb) - skb_mac_header(skb);
@@ -216,15 +241,23 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 		}
 	}
 
+	skb2->skb_iif = skb->dev->ifindex;
+	skb2->dev = dev;
+
 	/* mirror is always swallowed */
-	if (tcf_mirred_is_act_redirect(m_eaction)) {
+	if (is_redirect) {
 		skb2->tc_redirected = 1;
 		skb2->tc_from_ingress = skb2->tc_at_ingress;
+
+		/* let's the caller reinsert the packet, if possible */
+		if (use_reinsert) {
+			res->ingress = want_ingress;
+			res->qstats = this_cpu_ptr(m->common.cpu_qstats);
+			return TC_ACT_REINSERT;
+		}
 	}
 
-	skb2->skb_iif = skb->dev->ifindex;
-	skb2->dev = dev;
-	if (!tcf_mirred_act_wants_ingress(m_eaction))
+	if (!want_ingress)
 		err = dev_queue_xmit(skb2);
 	else
 		err = netif_receive_skb(skb2);
@@ -232,7 +265,7 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
 	if (err) {
 out:
 		qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
-		if (tcf_mirred_is_act_redirect(m_eaction))
+		if (is_redirect)
 			retval = TC_ACT_SHOT;
 	}
 
-- 
2.17.1

Re: [PATCH net-next v4 4/4] act_mirred: use TC_ACT_REINSERT when possible

[Cong Wang]

On Thu, Jul 26, 2018 at 7:35 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> When mirred is invoked from the ingress path, and it wants to redirect
> the processed packet, it can now use the TC_ACT_REINSERT action,
> filling the tcf_result accordingly, and avoiding a per packet
> skb_clone().
>
> Overall this gives a ~10% improvement in forwarding performance for the
> TC S/W data path and TC S/W performances are now comparable to the
> kernel openvswitch datapath.
>
> v1 -> v2: use ACT_MIRRED instead of ACT_REDIRECT
> v2 -> v3: updated after action rename, fixed typo into the commit
>         message
> v3 -> v4: updated again after action rename, added more comments to
>         the code (JiriP), skip the optimization if the control action
>         need to touch the tcf_result (Paolo)
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Overall it looks good to me now.

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Thanks!

Re: [PATCH net-next v4 4/4] act_mirred: use TC_ACT_REINSERT when possible

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4288 bytes --]

Hi Paolo,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Paolo-Abeni/TC-refactor-act_mirred-packets-re-injection/20180729-102154
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.1.0 make.cross ARCH=xtensa 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   net/sched/act_mirred.c: In function 'tcf_mirred':
>> net/sched/act_mirred.c:268:6: warning: 'is_redirect' may be used uninitialized in this function [-Wmaybe-uninitialized]
      if (is_redirect)
         ^

vim +/is_redirect +268 net/sched/act_mirred.c

   182	
   183	static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
   184			      struct tcf_result *res)
   185	{
   186		struct tcf_mirred *m = to_mirred(a);
   187		struct sk_buff *skb2 = skb;
   188		bool m_mac_header_xmit;
   189		struct net_device *dev;
   190		int retval, err = 0;
   191		bool use_reinsert;
   192		bool want_ingress;
   193		bool is_redirect;
   194		int m_eaction;
   195		int mac_len;
   196	
   197		tcf_lastuse_update(&m->tcf_tm);
   198		bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
   199	
   200		m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
   201		m_eaction = READ_ONCE(m->tcfm_eaction);
   202		retval = READ_ONCE(m->tcf_action);
   203		dev = rcu_dereference_bh(m->tcfm_dev);
   204		if (unlikely(!dev)) {
   205			pr_notice_once("tc mirred: target device is gone\n");
   206			goto out;
   207		}
   208	
   209		if (unlikely(!(dev->flags & IFF_UP))) {
   210			net_notice_ratelimited("tc mirred to Houston: device %s is down\n",
   211					       dev->name);
   212			goto out;
   213		}
   214	
   215		/* we could easily avoid the clone only if called by ingress and clsact;
   216		 * since we can't easily detect the clsact caller, skip clone only for
   217		 * ingress - that covers the TC S/W datapath.
   218		 */
   219		is_redirect = tcf_mirred_is_act_redirect(m_eaction);
   220		use_reinsert = skb_at_tc_ingress(skb) && is_redirect &&
   221			       tcf_mirred_can_reinsert(retval);
   222		if (!use_reinsert) {
   223			skb2 = skb_clone(skb, GFP_ATOMIC);
   224			if (!skb2)
   225				goto out;
   226		}
   227	
   228		/* If action's target direction differs than filter's direction,
   229		 * and devices expect a mac header on xmit, then mac push/pull is
   230		 * needed.
   231		 */
   232		want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
   233		if (skb_at_tc_ingress(skb) != want_ingress && m_mac_header_xmit) {
   234			if (!skb_at_tc_ingress(skb)) {
   235				/* caught at egress, act ingress: pull mac */
   236				mac_len = skb_network_header(skb) - skb_mac_header(skb);
   237				skb_pull_rcsum(skb2, mac_len);
   238			} else {
   239				/* caught at ingress, act egress: push mac */
   240				skb_push_rcsum(skb2, skb->mac_len);
   241			}
   242		}
   243	
   244		skb2->skb_iif = skb->dev->ifindex;
   245		skb2->dev = dev;
   246	
   247		/* mirror is always swallowed */
   248		if (is_redirect) {
   249			skb2->tc_redirected = 1;
   250			skb2->tc_from_ingress = skb2->tc_at_ingress;
   251	
   252			/* let's the caller reinsert the packet, if possible */
   253			if (use_reinsert) {
   254				res->ingress = want_ingress;
   255				res->qstats = this_cpu_ptr(m->common.cpu_qstats);
   256				return TC_ACT_REINSERT;
   257			}
   258		}
   259	
   260		if (!want_ingress)
   261			err = dev_queue_xmit(skb2);
   262		else
   263			err = netif_receive_skb(skb2);
   264	
   265		if (err) {
   266	out:
   267			qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
 > 268			if (is_redirect)
   269				retval = TC_ACT_SHOT;
   270		}
   271	
   272		return retval;
   273	}
   274	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54195 bytes --]