* [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
@ 2021-01-19 8:31 wenxu
2021-01-19 20:03 ` Marcelo Ricardo Leitner
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: wenxu @ 2021-01-19 8:31 UTC (permalink / raw)
To: marcelo.leitner, jhs, xiyou.wangcong; +Cc: netdev
From: wenxu <wenxu@ucloud.cn>
This patch add the TCA_FLOWER_KEY_CT_FLAGS_INVALID flag to
match the ct_state with invalid for conntrack.
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
v2: initialize post_ct right on the declaration
include/linux/skbuff.h | 4 ++--
include/net/sch_generic.h | 1 +
include/uapi/linux/pkt_cls.h | 1 +
net/core/dev.c | 2 ++
net/core/flow_dissector.c | 13 +++++++++----
net/sched/act_ct.c | 1 +
net/sched/cls_flower.c | 4 +++-
7 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c9568cf..e22ccf0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1353,8 +1353,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
skb_flow_dissect_ct(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container,
- u16 *ctinfo_map,
- size_t mapsize);
+ u16 *ctinfo_map, size_t mapsize,
+ bool post_ct);
void
skb_flow_dissect_tunnel_info(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 639e465..e7bee99 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -388,6 +388,7 @@ struct qdisc_skb_cb {
#define QDISC_CB_PRIV_LEN 20
unsigned char data[QDISC_CB_PRIV_LEN];
u16 mru;
+ bool post_ct;
};
typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv);
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index ee95f42..709668e 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -591,6 +591,7 @@ enum {
TCA_FLOWER_KEY_CT_FLAGS_ESTABLISHED = 1 << 1, /* Part of an existing connection. */
TCA_FLOWER_KEY_CT_FLAGS_RELATED = 1 << 2, /* Related to an established connection. */
TCA_FLOWER_KEY_CT_FLAGS_TRACKED = 1 << 3, /* Conntrack has occurred. */
+ TCA_FLOWER_KEY_CT_FLAGS_INVALID = 1 << 4, /* Conntrack is invalid. */
};
enum {
diff --git a/net/core/dev.c b/net/core/dev.c
index bae35c1..9dce3f7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3878,6 +3878,7 @@ int dev_loopback_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
/* qdisc_skb_cb(skb)->pkt_len was already set by the caller. */
qdisc_skb_cb(skb)->mru = 0;
+ qdisc_skb_cb(skb)->post_ct = false;
mini_qdisc_bstats_cpu_update(miniq, skb);
switch (tcf_classify(skb, miniq->filter_list, &cl_res, false)) {
@@ -4960,6 +4961,7 @@ static __latent_entropy void net_tx_action(struct softirq_action *h)
qdisc_skb_cb(skb)->pkt_len = skb->len;
qdisc_skb_cb(skb)->mru = 0;
+ qdisc_skb_cb(skb)->post_ct = false;
skb->tc_at_ingress = 1;
mini_qdisc_bstats_cpu_update(miniq, skb);
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 2d70ded..c565c7a 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
void
skb_flow_dissect_ct(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
- void *target_container,
- u16 *ctinfo_map,
- size_t mapsize)
+ void *target_container, u16 *ctinfo_map,
+ size_t mapsize, bool post_ct)
{
#if IS_ENABLED(CONFIG_NF_CONNTRACK)
struct flow_dissector_key_ct *key;
@@ -251,13 +250,19 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
return;
ct = nf_ct_get(skb, &ctinfo);
- if (!ct)
+ if (!ct && !post_ct)
return;
key = skb_flow_dissector_target(flow_dissector,
FLOW_DISSECTOR_KEY_CT,
target_container);
+ if (!ct) {
+ key->ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
+ TCA_FLOWER_KEY_CT_FLAGS_INVALID;
+ return;
+ }
+
if (ctinfo < mapsize)
key->ct_state = ctinfo_map[ctinfo];
#if IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 83a5c67..b344207 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1030,6 +1030,7 @@ static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
out:
tcf_action_update_bstats(&c->common, skb);
+ qdisc_skb_cb(skb)->post_ct = true;
if (defrag)
qdisc_skb_cb(skb)->pkt_len = skb->len;
return retval;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 1319986..0dcb5a0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -302,6 +302,7 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
struct cls_fl_head *head = rcu_dereference_bh(tp->root);
+ bool post_ct = qdisc_skb_cb(skb)->post_ct;
struct fl_flow_key skb_key;
struct fl_flow_mask *mask;
struct cls_fl_filter *f;
@@ -318,7 +319,8 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp,
skb_flow_dissect_tunnel_info(skb, &mask->dissector, &skb_key);
skb_flow_dissect_ct(skb, &mask->dissector, &skb_key,
fl_ct_info_to_flower_map,
- ARRAY_SIZE(fl_ct_info_to_flower_map));
+ ARRAY_SIZE(fl_ct_info_to_flower_map),
+ post_ct);
skb_flow_dissect_hash(skb, &mask->dissector, &skb_key);
skb_flow_dissect(skb, &mask->dissector, &skb_key, 0);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-19 8:31 [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support wenxu
@ 2021-01-19 20:03 ` Marcelo Ricardo Leitner
2021-01-20 22:18 ` Cong Wang
2021-01-21 6:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 7+ messages in thread
From: Marcelo Ricardo Leitner @ 2021-01-19 20:03 UTC (permalink / raw)
To: wenxu; +Cc: jhs, xiyou.wangcong, netdev
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
The patch looks good to me, just some side comments below.
On Tue, Jan 19, 2021 at 04:31:50PM +0800, wenxu@ucloud.cn wrote:
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3878,6 +3878,7 @@ int dev_loopback_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
I don't know why, but your patches often have function names here that
are not accurate.
>
> /* qdisc_skb_cb(skb)->pkt_len was already set by the caller. */
> qdisc_skb_cb(skb)->mru = 0;
> + qdisc_skb_cb(skb)->post_ct = false;
> mini_qdisc_bstats_cpu_update(miniq, skb);
>
> switch (tcf_classify(skb, miniq->filter_list, &cl_res, false)) {
> @@ -4960,6 +4961,7 @@ static __latent_entropy void net_tx_action(struct softirq_action *h)
Here as well.
>
> qdisc_skb_cb(skb)->pkt_len = skb->len;
> qdisc_skb_cb(skb)->mru = 0;
> + qdisc_skb_cb(skb)->post_ct = false;
> skb->tc_at_ingress = 1;
> mini_qdisc_bstats_cpu_update(miniq, skb);
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 2d70ded..c565c7a 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
Here, I would expect to see a label/function name just before the
skb_flow_dissect_ct definition. But that's
skb_flow_dissect_set_enc_addr_type. skb_flow_dissect_meta is still one
other function up.
> void
> skb_flow_dissect_ct(const struct sk_buff *skb,
> struct flow_dissector *flow_dissector,
> - void *target_container,
> - u16 *ctinfo_map,
> - size_t mapsize)
> + void *target_container, u16 *ctinfo_map,
> + size_t mapsize, bool post_ct)
> {
> #if IS_ENABLED(CONFIG_NF_CONNTRACK)
> struct flow_dissector_key_ct *key;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-19 8:31 [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support wenxu
2021-01-19 20:03 ` Marcelo Ricardo Leitner
@ 2021-01-20 22:18 ` Cong Wang
2021-01-20 23:40 ` Marcelo Ricardo Leitner
2021-01-21 6:20 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 7+ messages in thread
From: Cong Wang @ 2021-01-20 22:18 UTC (permalink / raw)
To: wenxu
Cc: Marcelo Ricardo Leitner, Jamal Hadi Salim,
Linux Kernel Network Developers
On Tue, Jan 19, 2021 at 12:33 AM <wenxu@ucloud.cn> wrote:
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 2d70ded..c565c7a 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
> void
> skb_flow_dissect_ct(const struct sk_buff *skb,
> struct flow_dissector *flow_dissector,
> - void *target_container,
> - u16 *ctinfo_map,
> - size_t mapsize)
> + void *target_container, u16 *ctinfo_map,
> + size_t mapsize, bool post_ct)
Why do you pass this boolean as a parameter when you
can just read it from qdisc_skb_cb(skb)?
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-20 22:18 ` Cong Wang
@ 2021-01-20 23:40 ` Marcelo Ricardo Leitner
2021-01-21 1:09 ` Cong Wang
0 siblings, 1 reply; 7+ messages in thread
From: Marcelo Ricardo Leitner @ 2021-01-20 23:40 UTC (permalink / raw)
To: Cong Wang; +Cc: wenxu, Jamal Hadi Salim, Linux Kernel Network Developers
On Wed, Jan 20, 2021 at 02:18:41PM -0800, Cong Wang wrote:
> On Tue, Jan 19, 2021 at 12:33 AM <wenxu@ucloud.cn> wrote:
> > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > index 2d70ded..c565c7a 100644
> > --- a/net/core/flow_dissector.c
> > +++ b/net/core/flow_dissector.c
> > @@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
> > void
> > skb_flow_dissect_ct(const struct sk_buff *skb,
> > struct flow_dissector *flow_dissector,
> > - void *target_container,
> > - u16 *ctinfo_map,
> > - size_t mapsize)
> > + void *target_container, u16 *ctinfo_map,
> > + size_t mapsize, bool post_ct)
>
> Why do you pass this boolean as a parameter when you
> can just read it from qdisc_skb_cb(skb)?
In this case, yes, but this way skb_flow_dissect_ct() can/is able to
not care about what the ->cb actually is. It could be called from
somewhere else too.
That's my rationale on it, not sure if wenxu thought the same.
Thanks,
Marcelo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-20 23:40 ` Marcelo Ricardo Leitner
@ 2021-01-21 1:09 ` Cong Wang
2021-01-21 2:37 ` wenxu
0 siblings, 1 reply; 7+ messages in thread
From: Cong Wang @ 2021-01-21 1:09 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: wenxu, Jamal Hadi Salim, Linux Kernel Network Developers
On Wed, Jan 20, 2021 at 3:40 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Wed, Jan 20, 2021 at 02:18:41PM -0800, Cong Wang wrote:
> > On Tue, Jan 19, 2021 at 12:33 AM <wenxu@ucloud.cn> wrote:
> > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > > index 2d70ded..c565c7a 100644
> > > --- a/net/core/flow_dissector.c
> > > +++ b/net/core/flow_dissector.c
> > > @@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
> > > void
> > > skb_flow_dissect_ct(const struct sk_buff *skb,
> > > struct flow_dissector *flow_dissector,
> > > - void *target_container,
> > > - u16 *ctinfo_map,
> > > - size_t mapsize)
> > > + void *target_container, u16 *ctinfo_map,
> > > + size_t mapsize, bool post_ct)
> >
> > Why do you pass this boolean as a parameter when you
> > can just read it from qdisc_skb_cb(skb)?
>
> In this case, yes, but this way skb_flow_dissect_ct() can/is able to
> not care about what the ->cb actually is. It could be called from
> somewhere else too.
This sounds reasonable, it is in net/core/ directory anyway,
so should be independent of tc even though cls_flower is its
only caller.
Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-21 1:09 ` Cong Wang
@ 2021-01-21 2:37 ` wenxu
0 siblings, 0 replies; 7+ messages in thread
From: wenxu @ 2021-01-21 2:37 UTC (permalink / raw)
To: Cong Wang, Marcelo Ricardo Leitner
Cc: Jamal Hadi Salim, Linux Kernel Network Developers
On 1/21/2021 9:09 AM, Cong Wang wrote:
> On Wed, Jan 20, 2021 at 3:40 PM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
>> On Wed, Jan 20, 2021 at 02:18:41PM -0800, Cong Wang wrote:
>>> On Tue, Jan 19, 2021 at 12:33 AM <wenxu@ucloud.cn> wrote:
>>>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>>> index 2d70ded..c565c7a 100644
>>>> --- a/net/core/flow_dissector.c
>>>> +++ b/net/core/flow_dissector.c
>>>> @@ -237,9 +237,8 @@ void skb_flow_dissect_meta(const struct sk_buff *skb,
>>>> void
>>>> skb_flow_dissect_ct(const struct sk_buff *skb,
>>>> struct flow_dissector *flow_dissector,
>>>> - void *target_container,
>>>> - u16 *ctinfo_map,
>>>> - size_t mapsize)
>>>> + void *target_container, u16 *ctinfo_map,
>>>> + size_t mapsize, bool post_ct)
>>> Why do you pass this boolean as a parameter when you
>>> can just read it from qdisc_skb_cb(skb)?
>> In this case, yes, but this way skb_flow_dissect_ct() can/is able to
>> not care about what the ->cb actually is. It could be called from
>> somewhere else too.
> This sounds reasonable, it is in net/core/ directory anyway,
> so should be independent of tc even though cls_flower is its
> only caller.
yes. This is the same what I think.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support
2021-01-19 8:31 [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support wenxu
2021-01-19 20:03 ` Marcelo Ricardo Leitner
2021-01-20 22:18 ` Cong Wang
@ 2021-01-21 6:20 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-01-21 6:20 UTC (permalink / raw)
To: wenxu; +Cc: marcelo.leitner, jhs, xiyou.wangcong, netdev
Hello:
This patch was applied to netdev/net-next.git (refs/heads/master):
On Tue, 19 Jan 2021 16:31:50 +0800 you wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> This patch add the TCA_FLOWER_KEY_CT_FLAGS_INVALID flag to
> match the ct_state with invalid for conntrack.
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
>
> [...]
Here is the summary with links:
- [v2,net-next] net/sched: cls_flower add CT_FLAGS_INVALID flag support
https://git.kernel.org/netdev/net-next/c/7baf2429a1a9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-21 6:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 8:31 [PATCH v2 net-next ] net/sched: cls_flower add CT_FLAGS_INVALID flag support wenxu
2021-01-19 20:03 ` Marcelo Ricardo Leitner
2021-01-20 22:18 ` Cong Wang
2021-01-20 23:40 ` Marcelo Ricardo Leitner
2021-01-21 1:09 ` Cong Wang
2021-01-21 2:37 ` wenxu
2021-01-21 6:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).