Re: [ovs-dev] [PATCH net v2 2/3] net/sched: flow_dissector: Fix matching on zone id for invalid conns

2021-12-14 Thread Paul Blakey via dev



On Sat, 11 Dec 2021, Jakub Kicinski wrote:

> On Thu, 9 Dec 2021 09:57:33 +0200 Paul Blakey wrote:
> > @@ -238,10 +239,12 @@ void
> >  skb_flow_dissect_ct(const struct sk_buff *skb,
> > struct flow_dissector *flow_dissector,
> > void *target_container, u16 *ctinfo_map,
> > -   size_t mapsize, bool post_ct)
> > +   size_t mapsize)
> >  {
> >  #if IS_ENABLED(CONFIG_NF_CONNTRACK)
> > +   bool post_ct = tc_skb_cb(skb)->post_ct;
> > struct flow_dissector_key_ct *key;
> > +   u16 zone = tc_skb_cb(skb)->zone;
> > enum ip_conntrack_info ctinfo;
> > struct nf_conn_labels *cl;
> > struct nf_conn *ct;
> > @@ -260,6 +263,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb,
> > if (!ct) {
> > key->ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
> > TCA_FLOWER_KEY_CT_FLAGS_INVALID;
> > +   key->ct_zone = zone;
> > return;
> > }
> >  
> 
> Why is flow dissector expecting skb cb to be TC now?
> Please keep the appropriate abstractions intact.
> 

Will do. Sending v3.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net v2 2/3] net/sched: flow_dissector: Fix matching on zone id for invalid conns

2021-12-11 Thread Cong Wang
On Fri, Dec 10, 2021 at 8:52 PM Jakub Kicinski  wrote:
>
> On Thu, 9 Dec 2021 09:57:33 +0200 Paul Blakey wrote:
> > @@ -238,10 +239,12 @@ void
> >  skb_flow_dissect_ct(const struct sk_buff *skb,
> >   struct flow_dissector *flow_dissector,
> >   void *target_container, u16 *ctinfo_map,
> > - size_t mapsize, bool post_ct)
> > + size_t mapsize)
> >  {
> >  #if IS_ENABLED(CONFIG_NF_CONNTRACK)
> > + bool post_ct = tc_skb_cb(skb)->post_ct;
> >   struct flow_dissector_key_ct *key;
> > + u16 zone = tc_skb_cb(skb)->zone;
> >   enum ip_conntrack_info ctinfo;
> >   struct nf_conn_labels *cl;
> >   struct nf_conn *ct;
> > @@ -260,6 +263,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb,
> >   if (!ct) {
> >   key->ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
> >   TCA_FLOWER_KEY_CT_FLAGS_INVALID;
> > + key->ct_zone = zone;
> >   return;
> >   }
> >
>
> Why is flow dissector expecting skb cb to be TC now?
> Please keep the appropriate abstractions intact.

Probably because only fl_classify() calls it, but I agree with you
on this point, this function is supposed to be TC independent.

Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH net v2 2/3] net/sched: flow_dissector: Fix matching on zone id for invalid conns

2021-12-10 Thread Jakub Kicinski
On Thu, 9 Dec 2021 09:57:33 +0200 Paul Blakey wrote:
> @@ -238,10 +239,12 @@ void
>  skb_flow_dissect_ct(const struct sk_buff *skb,
>   struct flow_dissector *flow_dissector,
>   void *target_container, u16 *ctinfo_map,
> - size_t mapsize, bool post_ct)
> + size_t mapsize)
>  {
>  #if IS_ENABLED(CONFIG_NF_CONNTRACK)
> + bool post_ct = tc_skb_cb(skb)->post_ct;
>   struct flow_dissector_key_ct *key;
> + u16 zone = tc_skb_cb(skb)->zone;
>   enum ip_conntrack_info ctinfo;
>   struct nf_conn_labels *cl;
>   struct nf_conn *ct;
> @@ -260,6 +263,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb,
>   if (!ct) {
>   key->ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
>   TCA_FLOWER_KEY_CT_FLAGS_INVALID;
> + key->ct_zone = zone;
>   return;
>   }
>  

Why is flow dissector expecting skb cb to be TC now?
Please keep the appropriate abstractions intact.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH net v2 2/3] net/sched: flow_dissector: Fix matching on zone id for invalid conns

2021-12-08 Thread Paul Blakey via dev
If ct rejects a flow, it removes the conntrack info from the skb.
act_ct sets the post_ct variable so the dissector will see this case
as an +tracked +invalid state, but the zone id is lost with the
conntrack info.

To restore the zone id on such cases, set the last executed zone,
via the tc control block, when passing ct, and read it back in the
dissector if there is no ct info on the skb (invalid connection).

Fixes: 7baf2429a1a9 ("net/sched: cls_flower add CT_FLAGS_INVALID flag support")
Signed-off-by: Paul Blakey 
---
 include/linux/skbuff.h| 3 +--
 include/net/pkt_sched.h   | 1 +
 net/core/flow_dissector.c | 6 +-
 net/sched/act_ct.c| 1 +
 net/sched/cls_flower.c| 5 ++---
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index c8cb7e697d47..155eb2ec54d8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1379,8 +1379,7 @@ void
 skb_flow_dissect_ct(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container,
-   u16 *ctinfo_map, size_t mapsize,
-   bool post_ct);
+   u16 *ctinfo_map, size_t mapsize);
 void
 skb_flow_dissect_tunnel_info(const struct sk_buff *skb,
 struct flow_dissector *flow_dissector,
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 05f18e81f3e8..9e71691c491b 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -198,6 +198,7 @@ struct tc_skb_cb {
 
u16 mru;
bool post_ct;
+   u16 zone; /* Only valid if post_ct = true */
 };
 
 static inline struct tc_skb_cb *tc_skb_cb(const struct sk_buff *skb)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 3255f57f5131..b52a4370162b 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -238,10 +239,12 @@ void
 skb_flow_dissect_ct(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
void *target_container, u16 *ctinfo_map,
-   size_t mapsize, bool post_ct)
+   size_t mapsize)
 {
 #if IS_ENABLED(CONFIG_NF_CONNTRACK)
+   bool post_ct = tc_skb_cb(skb)->post_ct;
struct flow_dissector_key_ct *key;
+   u16 zone = tc_skb_cb(skb)->zone;
enum ip_conntrack_info ctinfo;
struct nf_conn_labels *cl;
struct nf_conn *ct;
@@ -260,6 +263,7 @@ skb_flow_dissect_ct(const struct sk_buff *skb,
if (!ct) {
key->ct_state = TCA_FLOWER_KEY_CT_FLAGS_TRACKED |
TCA_FLOWER_KEY_CT_FLAGS_INVALID;
+   key->ct_zone = zone;
return;
}
 
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 98e248b9c0b1..ab3591408419 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1049,6 +1049,7 @@ static int tcf_ct_act(struct sk_buff *skb, const struct 
tc_action *a,
skb_push_rcsum(skb, nh_ofs);
 
tc_skb_cb(skb)->post_ct = true;
+   tc_skb_cb(skb)->zone = p->zone;
 out_clear:
if (defrag)
qdisc_skb_cb(skb)->pkt_len = skb->len;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9782b93db1b3..c1a017822c6e 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -310,7 +310,6 @@ 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 = tc_skb_cb(skb)->post_ct;
struct fl_flow_key skb_key;
struct fl_flow_mask *mask;
struct cls_fl_filter *f;
@@ -327,8 +326,8 @@ static int fl_classify(struct sk_buff *skb, const struct 
tcf_proto *tp,
skb_flow_dissect_tunnel_info(skb, >dissector, _key);
skb_flow_dissect_ct(skb, >dissector, _key,
fl_ct_info_to_flower_map,
-   ARRAY_SIZE(fl_ct_info_to_flower_map),
-   post_ct);
+   ARRAY_SIZE(fl_ct_info_to_flower_map));
+
skb_flow_dissect_hash(skb, >dissector, _key);
skb_flow_dissect(skb, >dissector, _key,
 FLOW_DISSECTOR_F_STOP_BEFORE_ENCAP);
-- 
2.30.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev