Thu, Jul 19, 2018 at 03:02:28PM CEST, pab...@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.
>
>v1 -> v2: do not touch TC_ACT_REDIRECT code path, introduce
> a new action type instead
>
>Signed-off-by: Paolo Abeni <pab...@redhat.com>
>---
> include/net/sch_generic.h | 19 +++++++++++++++++++
> include/uapi/linux/pkt_cls.h | 3 ++-
> net/core/dev.c | 4 ++++
> net/sched/act_api.c | 6 ++++--
> 4 files changed, 29 insertions(+), 3 deletions(-)
>
>diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>index 056dc1083aa3..667d7b66fee2 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_MIRRED action */
>+ struct {
>+ bool ingress;
>+ struct gnet_stats_queue *qstats;
>+ };
> };
> };
>
>@@ -1091,4 +1097,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_redirect(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/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>index 7cdd62b51106..1a5e8a3217f3 100644
>--- a/include/uapi/linux/pkt_cls.h
>+++ b/include/uapi/linux/pkt_cls.h
>@@ -45,7 +45,8 @@ enum {
> * the skb and act like everything
> * is alright.
> */
>-#define TC_ACT_LAST TC_ACT_TRAP
>+#define TC_ACT_MIRRED 9
>+#define TC_ACT_LAST TC_ACT_MIRRED
>
> /* There is a special kind of actions called "extended actions",
> * which need a value parameter. These have a local opcode located in
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 14a748ee8cc9..3822f29d730f 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -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_MIRRED:
>+ /* this does not scrub the packet, and updates stats on error */
>+ skb_tc_redirect(skb, &cl_res);
REDIRECT does skb_do_redirect and MIRRED does skb_tc_redirect.
Confusing. I agree with Cong that the name is not correct.
>+ return NULL;
> default:
> break;
> }
>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>index f6438f246dab..029302e2813e 100644
>--- a/net/sched/act_api.c
>+++ b/net/sched/act_api.c
>@@ -895,8 +895,10 @@ struct tc_action *tcf_action_init_1(struct net *net,
>struct tcf_proto *tp,
> }
> }
>
>- if (a->tcfa_action == TC_ACT_REDIRECT) {
>- net_warn_ratelimited("TC_ACT_REDIRECT can't be used directly");
>+ if (a->tcfa_action == TC_ACT_REDIRECT ||
>+ a->tcfa_action == TC_ACT_MIRRED) {
>+ net_warn_ratelimited("action %d can't be used directly",
>+ a->tcfa_action);
> a->tcfa_action = TC_ACT_LAST + 1;
> }
>
>--
>2.17.1
>
