| Message ID | 1506500194-17637-3-git-send-email-simon.horman@netronome.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net/sched: support tunnel options in cls_flower and act_tunnel_key | expand |
Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: >Allow matching on options in tunnel headers. >This makes use of existing tunnel metadata support. > >Options are a bytestring of up to 256 bytes. >Tunnel implementations may support less or more options, >or no options at all. > >e.g. > # ip link add name geneve0 type geneve dstport 0 external > # tc qdisc add dev geneve0 ingress > # tc filter add dev geneve0 protocol ip parent ffff: \ > flower \ > enc_src_ip 10.0.99.192 \ > enc_dst_ip 10.0.99.193 \ > enc_key_id 11 \ > enc_opts 0102800100800020/fffffffffffffff0 \ > ip_proto udp \ > action mirred egress redirect dev eth1 > >Signed-off-by: Simon Horman <simon.horman@netronome.com> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > >--- >v2 >* Correct example which was incorrectly described setting rather > than matching tunnel options >--- > include/net/flow_dissector.h | 13 +++++++++++++ > include/uapi/linux/pkt_cls.h | 3 +++ > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++- > 3 files changed, 50 insertions(+), 1 deletion(-) > >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h >index fc3dce730a6b..43f98bf0b349 100644 >--- a/include/net/flow_dissector.h >+++ b/include/net/flow_dissector.h >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip { > __u8 ttl; > }; > >+/** >+ * struct flow_dissector_key_enc_opts: >+ * @data: data >+ * @len: len >+ */ >+struct flow_dissector_key_enc_opts { >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here >+ * but seems difficult to #include >+ */ >+ u8 len; >+}; >+ > enum flow_dissector_key_id { > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ I don't see the actual dissection implementation. Where is it? Did you test the patchset?
On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > >Allow matching on options in tunnel headers. > >This makes use of existing tunnel metadata support. > > > >Options are a bytestring of up to 256 bytes. > >Tunnel implementations may support less or more options, > >or no options at all. > > > >e.g. > > # ip link add name geneve0 type geneve dstport 0 external > > # tc qdisc add dev geneve0 ingress > > # tc filter add dev geneve0 protocol ip parent ffff: \ > > flower \ > > enc_src_ip 10.0.99.192 \ > > enc_dst_ip 10.0.99.193 \ > > enc_key_id 11 \ > > enc_opts 0102800100800020/fffffffffffffff0 \ > > ip_proto udp \ > > action mirred egress redirect dev eth1 > > > >Signed-off-by: Simon Horman <simon.horman@netronome.com> > >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > > >--- > >v2 > >* Correct example which was incorrectly described setting rather > > than matching tunnel options > >--- > > include/net/flow_dissector.h | 13 +++++++++++++ > > include/uapi/linux/pkt_cls.h | 3 +++ > > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++- > > 3 files changed, 50 insertions(+), 1 deletion(-) > > > >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h > >index fc3dce730a6b..43f98bf0b349 100644 > >--- a/include/net/flow_dissector.h > >+++ b/include/net/flow_dissector.h > >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip { > > __u8 ttl; > > }; > > > >+/** > >+ * struct flow_dissector_key_enc_opts: > >+ * @data: data > >+ * @len: len > >+ */ > >+struct flow_dissector_key_enc_opts { > >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here > >+ * but seems difficult to #include > >+ */ > >+ u8 len; > >+}; > >+ > > enum flow_dissector_key_id { > > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > > I don't see the actual dissection implementation. Where is it? > Did you test the patchset? Yes, I did test it. But it is also possible something went astray along the way and I will retest. I think that the code you are looking for is in fl_classify() in this patch.
Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: >> >Allow matching on options in tunnel headers. >> >This makes use of existing tunnel metadata support. >> > >> >Options are a bytestring of up to 256 bytes. >> >Tunnel implementations may support less or more options, >> >or no options at all. >> > >> >e.g. >> > # ip link add name geneve0 type geneve dstport 0 external >> > # tc qdisc add dev geneve0 ingress >> > # tc filter add dev geneve0 protocol ip parent ffff: \ >> > flower \ >> > enc_src_ip 10.0.99.192 \ >> > enc_dst_ip 10.0.99.193 \ >> > enc_key_id 11 \ >> > enc_opts 0102800100800020/fffffffffffffff0 \ >> > ip_proto udp \ >> > action mirred egress redirect dev eth1 >> > >> >Signed-off-by: Simon Horman <simon.horman@netronome.com> >> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> >> > >> >--- >> >v2 >> >* Correct example which was incorrectly described setting rather >> > than matching tunnel options >> >--- >> > include/net/flow_dissector.h | 13 +++++++++++++ >> > include/uapi/linux/pkt_cls.h | 3 +++ >> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++- >> > 3 files changed, 50 insertions(+), 1 deletion(-) >> > >> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h >> >index fc3dce730a6b..43f98bf0b349 100644 >> >--- a/include/net/flow_dissector.h >> >+++ b/include/net/flow_dissector.h >> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip { >> > __u8 ttl; >> > }; >> > >> >+/** >> >+ * struct flow_dissector_key_enc_opts: >> >+ * @data: data >> >+ * @len: len >> >+ */ >> >+struct flow_dissector_key_enc_opts { >> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here >> >+ * but seems difficult to #include >> >+ */ >> >+ u8 len; >> >+}; >> >+ >> > enum flow_dissector_key_id { >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ >> >> I don't see the actual dissection implementation. Where is it? >> Did you test the patchset? > >Yes, I did test it. But it is also possible something went astray along the >way and I will retest. > >I think that the code you are looking for is in >fl_classify() in this patch. The dissection should be done in the flow_dissector. That's the whole point in having it generic. You should move it there.
On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: > >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > >> >Allow matching on options in tunnel headers. > >> >This makes use of existing tunnel metadata support. > >> > > >> >Options are a bytestring of up to 256 bytes. > >> >Tunnel implementations may support less or more options, > >> >or no options at all. > >> > > >> >e.g. > >> > # ip link add name geneve0 type geneve dstport 0 external > >> > # tc qdisc add dev geneve0 ingress > >> > # tc filter add dev geneve0 protocol ip parent ffff: \ > >> > flower \ > >> > enc_src_ip 10.0.99.192 \ > >> > enc_dst_ip 10.0.99.193 \ > >> > enc_key_id 11 \ > >> > enc_opts 0102800100800020/fffffffffffffff0 \ > >> > ip_proto udp \ > >> > action mirred egress redirect dev eth1 > >> > > >> >Signed-off-by: Simon Horman <simon.horman@netronome.com> > >> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > >> > > >> >--- > >> >v2 > >> >* Correct example which was incorrectly described setting rather > >> > than matching tunnel options > >> >--- > >> > include/net/flow_dissector.h | 13 +++++++++++++ > >> > include/uapi/linux/pkt_cls.h | 3 +++ > >> > net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++- > >> > 3 files changed, 50 insertions(+), 1 deletion(-) > >> > > >> >diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h > >> >index fc3dce730a6b..43f98bf0b349 100644 > >> >--- a/include/net/flow_dissector.h > >> >+++ b/include/net/flow_dissector.h > >> >@@ -183,6 +183,18 @@ struct flow_dissector_key_ip { > >> > __u8 ttl; > >> > }; > >> > > >> >+/** > >> >+ * struct flow_dissector_key_enc_opts: > >> >+ * @data: data > >> >+ * @len: len > >> >+ */ > >> >+struct flow_dissector_key_enc_opts { > >> >+ u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here > >> >+ * but seems difficult to #include > >> >+ */ > >> >+ u8 len; > >> >+}; > >> >+ > >> > enum flow_dissector_key_id { > >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > >> > >> I don't see the actual dissection implementation. Where is it? > >> Did you test the patchset? > > > >Yes, I did test it. But it is also possible something went astray along the > >way and I will retest. > > > >I think that the code you are looking for is in > >fl_classify() in this patch. > > The dissection should be done in the flow_dissector. That's the whole > point in having it generic. You should move it there. > Thanks, will do.
On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: > >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: ... > >> > enum flow_dissector_key_id { > >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > >> > >> I don't see the actual dissection implementation. Where is it? > >> Did you test the patchset? > > > >Yes, I did test it. But it is also possible something went astray along the > >way and I will retest. > > > >I think that the code you are looking for is in > >fl_classify() in this patch. > > The dissection should be done in the flow_dissector. That's the whole > point in having it generic. You should move it there. Coming back to this after lunch, I believe what I have done in this patch is consistent with handling of other enc fields, which are set in fl_classify() rather than the dissector. In particular the ip_tunnel_info, which is used by this patch, is already used in fl_classify(). Without this patch I see: static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, struct tcf_result *res) { ... struct ip_tunnel_info *info; ... info = skb_tunnel_info(skb); if (info) { struct ip_tunnel_key *key = &info->key; switch (ip_tunnel_info_af(info)) { case AF_INET: skb_key.enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS; skb_key.enc_ipv4.src = key->u.ipv4.src; skb_key.enc_ipv4.dst = key->u.ipv4.dst; break; case AF_INET6: skb_key.enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS; skb_key.enc_ipv6.src = key->u.ipv6.src; skb_key.enc_ipv6.dst = key->u.ipv6.dst; break; } skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id); skb_key.enc_tp.src = key->tp_src; skb_key.enc_tp.dst = key->tp_dst; } ... } This patch adds the following inside the if() clause above: if (info->options_len) { skb_key.enc_opts.len = info->options_len; ip_tunnel_info_opts_get(skb_key.enc_opts.data, info); }
Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > >... > >> >> > enum flow_dissector_key_id { >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ >> >> >> >> I don't see the actual dissection implementation. Where is it? >> >> Did you test the patchset? >> > >> >Yes, I did test it. But it is also possible something went astray along the >> >way and I will retest. >> > >> >I think that the code you are looking for is in >> >fl_classify() in this patch. >> >> The dissection should be done in the flow_dissector. That's the whole >> point in having it generic. You should move it there. > >Coming back to this after lunch, I believe what I have done in this patch >is consistent with handling of other enc fields, which are set in >fl_classify() rather than the dissector. In particular the ip_tunnel_info, >which is used by this patch, is already used in fl_classify(). That means the current code is wrong. The dissection should be done in flow_dissector, not in fl_classify. > >Without this patch I see: > > >static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, > struct tcf_result *res) >{ > ... > struct ip_tunnel_info *info; > > ... > > info = skb_tunnel_info(skb); > if (info) { > struct ip_tunnel_key *key = &info->key; > > switch (ip_tunnel_info_af(info)) { > case AF_INET: > skb_key.enc_control.addr_type = > FLOW_DISSECTOR_KEY_IPV4_ADDRS; > skb_key.enc_ipv4.src = key->u.ipv4.src; > skb_key.enc_ipv4.dst = key->u.ipv4.dst; > break; > case AF_INET6: > skb_key.enc_control.addr_type = > FLOW_DISSECTOR_KEY_IPV6_ADDRS; > skb_key.enc_ipv6.src = key->u.ipv6.src; > skb_key.enc_ipv6.dst = key->u.ipv6.dst; > break; > } > > skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id); > skb_key.enc_tp.src = key->tp_src; > skb_key.enc_tp.dst = key->tp_dst; > } > > ... >} > >This patch adds the following inside the if() clause above: > > if (info->options_len) { > skb_key.enc_opts.len = info->options_len; > ip_tunnel_info_opts_get(skb_key.enc_opts.data, info); > } > >
On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: > >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: > >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: > >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > > > >... > > > >> >> > enum flow_dissector_key_id { > >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > >> >> > >> >> I don't see the actual dissection implementation. Where is it? > >> >> Did you test the patchset? > >> > > >> >Yes, I did test it. But it is also possible something went astray along the > >> >way and I will retest. > >> > > >> >I think that the code you are looking for is in > >> >fl_classify() in this patch. > >> > >> The dissection should be done in the flow_dissector. That's the whole > >> point in having it generic. You should move it there. > > > >Coming back to this after lunch, I believe what I have done in this patch > >is consistent with handling of other enc fields, which are set in > >fl_classify() rather than the dissector. In particular the ip_tunnel_info, > >which is used by this patch, is already used in fl_classify(). > > That means the current code is wrong. The dissection should be done in > flow_dissector, not in fl_classify. Would an better approach be to move the fl_classify() below into, say, skb_flow_dissect_tunnel_info() and call that from fl_classify(). The reason I suggest this rather than moving the code into __skb_flow_dissect() is that currently flower assumes that tunnel_info is used if present. While I assume other users of () assume tunnel_info is not used even if present. > >Without this patch I see: > > > > > >static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, > > struct tcf_result *res) > >{ > > ... > > struct ip_tunnel_info *info; > > > > ... > > > > info = skb_tunnel_info(skb); > > if (info) { > > struct ip_tunnel_key *key = &info->key; > > > > switch (ip_tunnel_info_af(info)) { > > case AF_INET: > > skb_key.enc_control.addr_type = > > FLOW_DISSECTOR_KEY_IPV4_ADDRS; > > skb_key.enc_ipv4.src = key->u.ipv4.src; > > skb_key.enc_ipv4.dst = key->u.ipv4.dst; > > break; > > case AF_INET6: > > skb_key.enc_control.addr_type = > > FLOW_DISSECTOR_KEY_IPV6_ADDRS; > > skb_key.enc_ipv6.src = key->u.ipv6.src; > > skb_key.enc_ipv6.dst = key->u.ipv6.dst; > > break; > > } > > > > skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id); > > skb_key.enc_tp.src = key->tp_src; > > skb_key.enc_tp.dst = key->tp_dst; > > } > > > > ... > >} > > > >This patch adds the following inside the if() clause above: > > > > if (info->options_len) { > > skb_key.enc_opts.len = info->options_len; > > ip_tunnel_info_opts_get(skb_key.enc_opts.data, info); > > } > > > >
Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote: >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: >> > >> >... >> > >> >> >> > enum flow_dissector_key_id { >> >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ >> >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { >> >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ >> >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ >> >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >> >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ >> >> >> >> >> >> I don't see the actual dissection implementation. Where is it? >> >> >> Did you test the patchset? >> >> > >> >> >Yes, I did test it. But it is also possible something went astray along the >> >> >way and I will retest. >> >> > >> >> >I think that the code you are looking for is in >> >> >fl_classify() in this patch. >> >> >> >> The dissection should be done in the flow_dissector. That's the whole >> >> point in having it generic. You should move it there. >> > >> >Coming back to this after lunch, I believe what I have done in this patch >> >is consistent with handling of other enc fields, which are set in >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info, >> >which is used by this patch, is already used in fl_classify(). >> >> That means the current code is wrong. The dissection should be done in >> flow_dissector, not in fl_classify. > >Would an better approach be to move the fl_classify() below into, say, >skb_flow_dissect_tunnel_info() and call that from fl_classify(). No. There is one flow dissection function and you just set it up in a way you need it. Makes no sense to me to split it up in any way. > >The reason I suggest this rather than moving the code into >__skb_flow_dissect() is that currently flower assumes that tunnel_info >is used if present. While I assume other users of () assume tunnel_info >is not used even if present. __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info only in case it is needed. > >> >Without this patch I see: >> > >> > >> >static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, >> > struct tcf_result *res) >> >{ >> > ... >> > struct ip_tunnel_info *info; >> > >> > ... >> > >> > info = skb_tunnel_info(skb); >> > if (info) { >> > struct ip_tunnel_key *key = &info->key; >> > >> > switch (ip_tunnel_info_af(info)) { >> > case AF_INET: >> > skb_key.enc_control.addr_type = >> > FLOW_DISSECTOR_KEY_IPV4_ADDRS; >> > skb_key.enc_ipv4.src = key->u.ipv4.src; >> > skb_key.enc_ipv4.dst = key->u.ipv4.dst; >> > break; >> > case AF_INET6: >> > skb_key.enc_control.addr_type = >> > FLOW_DISSECTOR_KEY_IPV6_ADDRS; >> > skb_key.enc_ipv6.src = key->u.ipv6.src; >> > skb_key.enc_ipv6.dst = key->u.ipv6.dst; >> > break; >> > } >> > >> > skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id); >> > skb_key.enc_tp.src = key->tp_src; >> > skb_key.enc_tp.dst = key->tp_dst; >> > } >> > >> > ... >> >} >> > >> >This patch adds the following inside the if() clause above: >> > >> > if (info->options_len) { >> > skb_key.enc_opts.len = info->options_len; >> > ip_tunnel_info_opts_get(skb_key.enc_opts.data, info); >> > } >> > >> >
On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote: > >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: > >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: > >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: > >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: > >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > >> > > >> >... > >> > > >> >> >> > enum flow_dissector_key_id { > >> >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > >> >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > >> >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > >> >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > >> >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >> >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > >> >> >> > >> >> >> I don't see the actual dissection implementation. Where is it? > >> >> >> Did you test the patchset? > >> >> > > >> >> >Yes, I did test it. But it is also possible something went astray along the > >> >> >way and I will retest. > >> >> > > >> >> >I think that the code you are looking for is in > >> >> >fl_classify() in this patch. > >> >> > >> >> The dissection should be done in the flow_dissector. That's the whole > >> >> point in having it generic. You should move it there. > >> > > >> >Coming back to this after lunch, I believe what I have done in this patch > >> >is consistent with handling of other enc fields, which are set in > >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info, > >> >which is used by this patch, is already used in fl_classify(). > >> > >> That means the current code is wrong. The dissection should be done in > >> flow_dissector, not in fl_classify. > > > >Would an better approach be to move the fl_classify() below into, say, > >skb_flow_dissect_tunnel_info() and call that from fl_classify(). > > No. There is one flow dissection function and you just set it up in a > way you need it. Makes no sense to me to split it up in any way. > > > > > >The reason I suggest this rather than moving the code into > >__skb_flow_dissect() is that currently flower assumes that tunnel_info > >is used if present. While I assume other users of () assume tunnel_info > >is not used even if present. > > __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info > only in case it is needed. Ok, do you think it is sufficient for __skb_flow_dissect to look at the tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may break flower as it look at the tunnel info unconditionally.
Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote: >On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote: >> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote: >> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: >> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: >> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: >> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: >> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: >> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: >> >> > >> >> >... >> >> > >> >> >> >> > enum flow_dissector_key_id { >> >> >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ >> >> >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { >> >> >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ >> >> >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ >> >> >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >> >> >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ >> >> >> >> >> >> >> >> I don't see the actual dissection implementation. Where is it? >> >> >> >> Did you test the patchset? >> >> >> > >> >> >> >Yes, I did test it. But it is also possible something went astray along the >> >> >> >way and I will retest. >> >> >> > >> >> >> >I think that the code you are looking for is in >> >> >> >fl_classify() in this patch. >> >> >> >> >> >> The dissection should be done in the flow_dissector. That's the whole >> >> >> point in having it generic. You should move it there. >> >> > >> >> >Coming back to this after lunch, I believe what I have done in this patch >> >> >is consistent with handling of other enc fields, which are set in >> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info, >> >> >which is used by this patch, is already used in fl_classify(). >> >> >> >> That means the current code is wrong. The dissection should be done in >> >> flow_dissector, not in fl_classify. >> > >> >Would an better approach be to move the fl_classify() below into, say, >> >skb_flow_dissect_tunnel_info() and call that from fl_classify(). >> >> No. There is one flow dissection function and you just set it up in a >> way you need it. Makes no sense to me to split it up in any way. >> >> >> > >> >The reason I suggest this rather than moving the code into >> >__skb_flow_dissect() is that currently flower assumes that tunnel_info >> >is used if present. While I assume other users of () assume tunnel_info >> >is not used even if present. >> >> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info >> only in case it is needed. > >Ok, do you think it is sufficient for __skb_flow_dissect to look at the >tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may >break flower as it look at the tunnel info unconditionally. yeah. When flower needs that, it will get that from the flow dissector. I don't see why it would break anything. Again, existing code is wrong: commit bc3103f1ed405de587fa43d8b0671e615505a700 Author: Amir Vadai <amir@vadai.me> Date: Thu Sep 8 16:23:47 2016 +0300 net/sched: cls_flower: Classify packet in ip tunnels The dissection has to be moved to flow dissector.
On Wed, Sep 27, 2017 at 04:00:11PM +0200, Jiri Pirko wrote: > Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote: > >On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote: > >> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote: > >> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: > >> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: > >> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: > >> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: > >> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: > >> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: > >> >> > > >> >> >... > >> >> > > >> >> >> >> > enum flow_dissector_key_id { > >> >> >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ > >> >> >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ > >> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { > >> >> >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ > >> >> >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ > >> >> >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ > >> >> >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ > >> >> >> >> > >> >> >> >> I don't see the actual dissection implementation. Where is it? > >> >> >> >> Did you test the patchset? > >> >> >> > > >> >> >> >Yes, I did test it. But it is also possible something went astray along the > >> >> >> >way and I will retest. > >> >> >> > > >> >> >> >I think that the code you are looking for is in > >> >> >> >fl_classify() in this patch. > >> >> >> > >> >> >> The dissection should be done in the flow_dissector. That's the whole > >> >> >> point in having it generic. You should move it there. > >> >> > > >> >> >Coming back to this after lunch, I believe what I have done in this patch > >> >> >is consistent with handling of other enc fields, which are set in > >> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info, > >> >> >which is used by this patch, is already used in fl_classify(). > >> >> > >> >> That means the current code is wrong. The dissection should be done in > >> >> flow_dissector, not in fl_classify. > >> > > >> >Would an better approach be to move the fl_classify() below into, say, > >> >skb_flow_dissect_tunnel_info() and call that from fl_classify(). > >> > >> No. There is one flow dissection function and you just set it up in a > >> way you need it. Makes no sense to me to split it up in any way. > >> > >> > >> > > >> >The reason I suggest this rather than moving the code into > >> >__skb_flow_dissect() is that currently flower assumes that tunnel_info > >> >is used if present. While I assume other users of () assume tunnel_info > >> >is not used even if present. > >> > >> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info > >> only in case it is needed. > > > >Ok, do you think it is sufficient for __skb_flow_dissect to look at the > >tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may > >break flower as it look at the tunnel info unconditionally. > > yeah. When flower needs that, it will get that from the flow dissector. > I don't see why it would break anything. Again, existing code is wrong: I understand that you think the existing code is wrong. But I also want to try not to add new bugs. I am concerned about the case where none of FLOW_DISSECTOR_KEY_ENC_* are set but flower currently dissects the tunnel info anyway. If I make dissection of tunnel info dependent on FLOW_DISSECTOR_KEY_ENC_* that may change things.
Wed, Sep 27, 2017 at 04:09:54PM CEST, simon.horman@netronome.com wrote: >On Wed, Sep 27, 2017 at 04:00:11PM +0200, Jiri Pirko wrote: >> Wed, Sep 27, 2017 at 03:50:44PM CEST, simon.horman@netronome.com wrote: >> >On Wed, Sep 27, 2017 at 03:47:50PM +0200, Jiri Pirko wrote: >> >> Wed, Sep 27, 2017 at 03:37:33PM CEST, simon.horman@netronome.com wrote: >> >> >On Wed, Sep 27, 2017 at 02:56:03PM +0200, Jiri Pirko wrote: >> >> >> Wed, Sep 27, 2017 at 02:52:06PM CEST, simon.horman@netronome.com wrote: >> >> >> >On Wed, Sep 27, 2017 at 01:08:22PM +0200, Jiri Pirko wrote: >> >> >> >> Wed, Sep 27, 2017 at 11:27:33AM CEST, simon.horman@netronome.com wrote: >> >> >> >> >On Wed, Sep 27, 2017 at 11:10:05AM +0200, Jiri Pirko wrote: >> >> >> >> >> Wed, Sep 27, 2017 at 10:16:34AM CEST, simon.horman@netronome.com wrote: >> >> >> > >> >> >> >... >> >> >> > >> >> >> >> >> > enum flow_dissector_key_id { >> >> >> >> >> > FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ >> >> >> >> >> > FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ >> >> >> >> >> >@@ -205,6 +217,7 @@ enum flow_dissector_key_id { >> >> >> >> >> > FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ >> >> >> >> >> > FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ >> >> >> >> >> > FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ >> >> >> >> >> >+ FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ >> >> >> >> >> >> >> >> >> >> I don't see the actual dissection implementation. Where is it? >> >> >> >> >> Did you test the patchset? >> >> >> >> > >> >> >> >> >Yes, I did test it. But it is also possible something went astray along the >> >> >> >> >way and I will retest. >> >> >> >> > >> >> >> >> >I think that the code you are looking for is in >> >> >> >> >fl_classify() in this patch. >> >> >> >> >> >> >> >> The dissection should be done in the flow_dissector. That's the whole >> >> >> >> point in having it generic. You should move it there. >> >> >> > >> >> >> >Coming back to this after lunch, I believe what I have done in this patch >> >> >> >is consistent with handling of other enc fields, which are set in >> >> >> >fl_classify() rather than the dissector. In particular the ip_tunnel_info, >> >> >> >which is used by this patch, is already used in fl_classify(). >> >> >> >> >> >> That means the current code is wrong. The dissection should be done in >> >> >> flow_dissector, not in fl_classify. >> >> > >> >> >Would an better approach be to move the fl_classify() below into, say, >> >> >skb_flow_dissect_tunnel_info() and call that from fl_classify(). >> >> >> >> No. There is one flow dissection function and you just set it up in a >> >> way you need it. Makes no sense to me to split it up in any way. >> >> >> >> >> >> > >> >> >The reason I suggest this rather than moving the code into >> >> >__skb_flow_dissect() is that currently flower assumes that tunnel_info >> >> >is used if present. While I assume other users of () assume tunnel_info >> >> >is not used even if present. >> >> >> >> __skb_flow_dissect should look at what caller wants, then check skb_tunnel_info >> >> only in case it is needed. >> > >> >Ok, do you think it is sufficient for __skb_flow_dissect to look at the >> >tunnel keys, say FLOW_DISSECTOR_KEY_ENC_*? I am a bit concerned this may >> >break flower as it look at the tunnel info unconditionally. >> >> yeah. When flower needs that, it will get that from the flow dissector. >> I don't see why it would break anything. Again, existing code is wrong: > >I understand that you think the existing code is wrong. >But I also want to try not to add new bugs. > >I am concerned about the case where none of FLOW_DISSECTOR_KEY_ENC_* are >set but flower currently dissects the tunnel info anyway. If I make >dissection of tunnel info dependent on FLOW_DISSECTOR_KEY_ENC_* >that may change things. If none of FLOW_DISSECTOR_KEY_ENC_* are set, flower does not care about the fields and therefore they are masked out by fl_set_masked_key. Otherwise it would be a bug is flower would match on something user did not specify.
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h index fc3dce730a6b..43f98bf0b349 100644 --- a/include/net/flow_dissector.h +++ b/include/net/flow_dissector.h @@ -183,6 +183,18 @@ struct flow_dissector_key_ip { __u8 ttl; }; +/** + * struct flow_dissector_key_enc_opts: + * @data: data + * @len: len + */ +struct flow_dissector_key_enc_opts { + u8 data[256]; /* Using IP_TUNNEL_OPTS_MAX is desired here + * but seems difficult to #include + */ + u8 len; +}; + enum flow_dissector_key_id { FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */ FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */ @@ -205,6 +217,7 @@ enum flow_dissector_key_id { FLOW_DISSECTOR_KEY_MPLS, /* struct flow_dissector_key_mpls */ FLOW_DISSECTOR_KEY_TCP, /* struct flow_dissector_key_tcp */ FLOW_DISSECTOR_KEY_IP, /* struct flow_dissector_key_ip */ + FLOW_DISSECTOR_KEY_ENC_OPTS, /* struct flow_dissector_key_enc_opts */ FLOW_DISSECTOR_KEY_MAX, }; diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h index d5e2bf68d0d4..7a09a28f21e0 100644 --- a/include/uapi/linux/pkt_cls.h +++ b/include/uapi/linux/pkt_cls.h @@ -467,6 +467,9 @@ enum { TCA_FLOWER_KEY_IP_TTL, /* u8 */ TCA_FLOWER_KEY_IP_TTL_MASK, /* u8 */ + TCA_FLOWER_KEY_ENC_OPTS, + TCA_FLOWER_KEY_ENC_OPTS_MASK, + __TCA_FLOWER_MAX, }; diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index d230cb4c8094..e72a17c46f07 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -51,6 +51,7 @@ struct fl_flow_key { struct flow_dissector_key_mpls mpls; struct flow_dissector_key_tcp tcp; struct flow_dissector_key_ip ip; + struct flow_dissector_key_enc_opts enc_opts; } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */ struct fl_flow_mask_range { @@ -181,6 +182,11 @@ static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, skb_key.enc_key_id.keyid = tunnel_id_to_key32(key->tun_id); skb_key.enc_tp.src = key->tp_src; skb_key.enc_tp.dst = key->tp_dst; + + if (info->options_len) { + skb_key.enc_opts.len = info->options_len; + ip_tunnel_info_opts_get(skb_key.enc_opts.data, info); + } } skb_key.indev_ifindex = skb->skb_iif; @@ -421,6 +427,8 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = { [TCA_FLOWER_KEY_IP_TOS_MASK] = { .type = NLA_U8 }, [TCA_FLOWER_KEY_IP_TTL] = { .type = NLA_U8 }, [TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NLA_U8 }, + [TCA_FLOWER_KEY_ENC_OPTS] = { .type = NLA_BINARY }, + [TCA_FLOWER_KEY_ENC_OPTS_MASK] = { .type = NLA_BINARY }, }; static void fl_set_key_val(struct nlattr **tb, @@ -712,6 +720,26 @@ static int fl_set_key(struct net *net, struct nlattr **tb, &mask->enc_tp.dst, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, sizeof(key->enc_tp.dst)); + if (tb[TCA_FLOWER_KEY_ENC_OPTS]) { + key->enc_opts.len = nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS]); + + if (key->enc_opts.len > sizeof(key->enc_opts.data)) + return -EINVAL; + + /* enc_opts is variable length. + * If present ensure the value and mask are the same length. + */ + if (tb[TCA_FLOWER_KEY_ENC_OPTS_MASK] && + nla_len(tb[TCA_FLOWER_KEY_ENC_OPTS_MASK]) != key->enc_opts.len) + return -EINVAL; + + mask->enc_opts.len = key->enc_opts.len; + fl_set_key_val(tb, key->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS, + mask->enc_opts.data, + TCA_FLOWER_KEY_ENC_OPTS_MASK, + key->enc_opts.len); + } + if (tb[TCA_FLOWER_KEY_FLAGS]) ret = fl_set_key_flags(tb, &key->control.flags, &mask->control.flags); @@ -804,6 +832,8 @@ static void fl_init_dissector(struct cls_fl_head *head, enc_control); FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt, FLOW_DISSECTOR_KEY_ENC_PORTS, enc_tp); + FL_KEY_SET_IF_MASKED(&mask->key, keys, cnt, + FLOW_DISSECTOR_KEY_ENC_OPTS, enc_opts); skb_flow_dissector_init(&head->dissector, keys, cnt); } @@ -1330,7 +1360,10 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, &mask->enc_tp.dst, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK, - sizeof(key->enc_tp.dst))) + sizeof(key->enc_tp.dst)) || + fl_dump_key_val(skb, key->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS, + mask->enc_opts.data, TCA_FLOWER_KEY_ENC_OPTS_MASK, + key->enc_opts.len)) goto nla_put_failure; if (fl_dump_key_flags(skb, key->control.flags, mask->control.flags))
Allow matching on options in tunnel headers. This makes use of existing tunnel metadata support. Options are a bytestring of up to 256 bytes. Tunnel implementations may support less or more options, or no options at all. e.g. # ip link add name geneve0 type geneve dstport 0 external # tc qdisc add dev geneve0 ingress # tc filter add dev geneve0 protocol ip parent ffff: \ flower \ enc_src_ip 10.0.99.192 \ enc_dst_ip 10.0.99.193 \ enc_key_id 11 \ enc_opts 0102800100800020/fffffffffffffff0 \ ip_proto udp \ action mirred egress redirect dev eth1 Signed-off-by: Simon Horman <simon.horman@netronome.com> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> --- v2 * Correct example which was incorrectly described setting rather than matching tunnel options --- include/net/flow_dissector.h | 13 +++++++++++++ include/uapi/linux/pkt_cls.h | 3 +++ net/sched/cls_flower.c | 35 ++++++++++++++++++++++++++++++++++- 3 files changed, 50 insertions(+), 1 deletion(-)