Re: [ovs-dev] [PATCH net-next] net: openvswitch: select vport upcall portid drectly

2019-11-06 Thread Or Gerlitz 
On Wed, Nov 6, 2019 at 3:04 PM  wrote:
> From: Tonghao Zhang 
>

drectly --> directly in the commit title
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] netdev-tc-offloads: Properly get the block id on flow del/get

2019-03-17 Thread Or Gerlitz 
Currnetly, when a tc flow is installed on a bond port using shared blocks,
we get these failures from the validator threads:

2019-03-17T10:02:58.919Z|13369|dpif(revalidator93)|WARN|system@ovs-system: 
failed to flow_del \
(No such file or directory) ufid:ebe2888b-9886-4835-a42e-c2911f6af6e8 
skb_priority(0),skb_mark(0),in_port(2), \

packet_type(ns=0,id=0),eth(src=e4:11:22:33:44:71,dst=24:8a:07:88:28:12),eth_type(0x0806),
 [..]

The block id must be retrieved from the device we got by ufid lookup and
not from the input to the related function, fix that for flow del and get.

While here, add the block id to existing debug print.

Fixes: 88dcf2aa8234 ('netdev-provider: add class op to get block_id')
Signed-off-by: Or Gerlitz 
Signed-off-by: Paul Blakey 
Reviewed-by: Roi Dayan 
---
 lib/netdev-tc-offloads.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index b33a79bb1..4b754bc82 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -1387,9 +1387,9 @@ netdev_tc_flow_get(struct netdev *netdev OVS_UNUSED,
 return -ifindex;
 }
 
-VLOG_DBG_RL(, "flow get (dev %s prio %d handle %d)",
-netdev_get_name(dev), prio, handle);
-block_id = get_block_id_from_netdev(netdev);
+block_id = get_block_id_from_netdev(dev);
+VLOG_DBG_RL(, "flow get (dev %s prio %d handle %d block_id %d)",
+netdev_get_name(dev), prio, handle, block_id);
 err = tc_get_flower(ifindex, prio, handle, , block_id);
 netdev_close(dev);
 if (err) {
@@ -1433,7 +1433,7 @@ netdev_tc_flow_del(struct netdev *netdev OVS_UNUSED,
 return -ifindex;
 }
 
-block_id = get_block_id_from_netdev(netdev);
+block_id = get_block_id_from_netdev(dev);
 
 if (stats) {
 memset(stats, 0, sizeof *stats);
-- 
2.17.2

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

[ovs-dev] [PATCH 1/2] lib/tc: Put the tunnel match fields as part of the tc/flower key struct

2018-09-06 Thread Or Gerlitz 
Move the tunnel match fields to be part of the tc/flower key structure.

This is pre-step for being able to apply masked match where needed.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 lib/netdev-tc-offloads.c | 50 
 lib/tc.c | 38 ++--
 lib/tc.h | 33 
 3 files changed, 61 insertions(+), 60 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 7bc745e..32d09d8 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -500,24 +500,24 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 }
 }
 
-if (flower->tunnel.tunnel) {
-match_set_tun_id(match, flower->tunnel.id);
-if (flower->tunnel.ipv4.ipv4_dst) {
-match_set_tun_src(match, flower->tunnel.ipv4.ipv4_src);
-match_set_tun_dst(match, flower->tunnel.ipv4.ipv4_dst);
-} else if (!is_all_zeros(>tunnel.ipv6.ipv6_dst,
-   sizeof flower->tunnel.ipv6.ipv6_dst)) {
-match_set_tun_ipv6_src(match, >tunnel.ipv6.ipv6_src);
-match_set_tun_ipv6_dst(match, >tunnel.ipv6.ipv6_dst);
+if (flower->tunnel) {
+match_set_tun_id(match, flower->key.tunnel.id);
+if (flower->key.tunnel.ipv4.ipv4_dst) {
+match_set_tun_src(match, flower->key.tunnel.ipv4.ipv4_src);
+match_set_tun_dst(match, flower->key.tunnel.ipv4.ipv4_dst);
+} else if (!is_all_zeros(>key.tunnel.ipv6.ipv6_dst,
+   sizeof flower->key.tunnel.ipv6.ipv6_dst)) {
+match_set_tun_ipv6_src(match, >key.tunnel.ipv6.ipv6_src);
+match_set_tun_ipv6_dst(match, >key.tunnel.ipv6.ipv6_dst);
 }
-if (flower->tunnel.tos) {
-match_set_tun_tos(match, flower->tunnel.tos);
+if (flower->key.tunnel.tos) {
+match_set_tun_tos(match, flower->key.tunnel.tos);
 }
-if (flower->tunnel.ttl) {
-match_set_tun_ttl(match, flower->tunnel.ttl);
+if (flower->key.tunnel.ttl) {
+match_set_tun_ttl(match, flower->key.tunnel.ttl);
 }
-if (flower->tunnel.tp_dst) {
-match_set_tun_tp_dst(match, flower->tunnel.tp_dst);
+if (flower->key.tunnel.tp_dst) {
+match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst);
 }
 }
 
@@ -964,16 +964,16 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 ntohll(tnl->tun_id),
 IP_ARGS(tnl->ip_src), IP_ARGS(tnl->ip_dst),
 ntohs(tnl->tp_src), ntohs(tnl->tp_dst));
-flower.tunnel.id = tnl->tun_id;
-flower.tunnel.ipv4.ipv4_src = tnl->ip_src;
-flower.tunnel.ipv4.ipv4_dst = tnl->ip_dst;
-flower.tunnel.ipv6.ipv6_src = tnl->ipv6_src;
-flower.tunnel.ipv6.ipv6_dst = tnl->ipv6_dst;
-flower.tunnel.tos = tnl->ip_tos;
-flower.tunnel.ttl = tnl->ip_ttl;
-flower.tunnel.tp_src = tnl->tp_src;
-flower.tunnel.tp_dst = tnl->tp_dst;
-flower.tunnel.tunnel = true;
+flower.key.tunnel.id = tnl->tun_id;
+flower.key.tunnel.ipv4.ipv4_src = tnl->ip_src;
+flower.key.tunnel.ipv4.ipv4_dst = tnl->ip_dst;
+flower.key.tunnel.ipv6.ipv6_src = tnl->ipv6_src;
+flower.key.tunnel.ipv6.ipv6_dst = tnl->ipv6_dst;
+flower.key.tunnel.tos = tnl->ip_tos;
+flower.key.tunnel.ttl = tnl->ip_ttl;
+flower.key.tunnel.tp_src = tnl->tp_src;
+flower.key.tunnel.tp_dst = tnl->tp_dst;
+flower.tunnel = true;
 }
 memset(>tunnel, 0, sizeof mask->tunnel);
 
diff --git a/lib/tc.c b/lib/tc.c
index bbc3823..22e72ee 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -393,34 +393,34 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 if (attrs[TCA_FLOWER_KEY_ENC_KEY_ID]) {
 ovs_be32 id = nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_KEY_ID]);
 
-flower->tunnel.id = be32_to_be64(id);
+flower->key.tunnel.id = be32_to_be64(id);
 }
 if (attrs[TCA_FLOWER_KEY_ENC_IPV4_SRC_MASK]) {
-flower->tunnel.ipv4.ipv4_src =
+flower->key.tunnel.ipv4.ipv4_src =
 nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_IPV4_SRC]);
 }
 if (attrs[TCA_FLOWER_KEY_ENC_IPV4_DST_MASK]) {
-flower->tunnel.ipv4.ipv4_dst =
+flower->key.tunnel.ipv4.ipv4_dst =
 nl_attr_get_be32(attrs[TCA_FLOWER_KEY_ENC_IPV4_DST]);
 }
 if (attrs[TCA_FLOWER_KEY_ENC_IPV6_SRC_MASK]) {
-flower->tunnel.ipv6.ipv6_src =
+flower->key.tunnel.ipv6.ipv6_src =
 nl_attr_get_in6_addr(attrs[TCA_FLOWER_KEY_ENC_IPV6_SRC]);
 }
 if (attrs[TCA_FLOWER_KEY_ENC_IPV6_

[ovs-dev] [PATCH 0/2] Fixes for TC DP matching on IP tunnels ttl/tos

2018-09-06 Thread Or Gerlitz 
The tc library wrongly programs the kernel tc stack to match
on tunnel ttl/tos also when not needed. Fix that by taking into
account the mask.

Or.

Or Gerlitz (2):
  lib/tc: Put the tunnel match fields as part of the tc/flower key struct
  lib/tc: Avoid matching on tunnel ttl or tos if not needed

 lib/netdev-tc-offloads.c | 55 ++--
 lib/tc.c | 54 +++
 lib/tc.h | 33 +++--
 3 files changed, 78 insertions(+), 64 deletions(-)

-- 
2.5.5

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

[ovs-dev] [PATCH 2/2] lib/tc: Avoid matching on tunnel ttl or tos if not needed

2018-09-06 Thread Or Gerlitz 
The tunnel ttl key is not masked when provided to the tc lib, hence we
wrongly attempted to match on it, when we got non zero ttl key with a zero
mask. Fix it by applying the mask. Use the same practice for the tunnel tos.

Fixes: dd83253e117c ('lib/tc: Support matching on ip tunnel tos and ttl')
Signed-off-by: Or Gerlitz 
Reported-by: Eli Britstein 
Reviewed-by: Roi Dayan 
---
 lib/netdev-tc-offloads.c |  9 +++--
 lib/tc.c | 16 
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 32d09d8..68f3bb4 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -511,10 +511,12 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_tun_ipv6_dst(match, >key.tunnel.ipv6.ipv6_dst);
 }
 if (flower->key.tunnel.tos) {
-match_set_tun_tos(match, flower->key.tunnel.tos);
+match_set_tun_tos_masked(match, flower->key.tunnel.tos,
+ flower->mask.tunnel.tos);
 }
 if (flower->key.tunnel.ttl) {
-match_set_tun_ttl(match, flower->key.tunnel.ttl);
+match_set_tun_ttl_masked(match, flower->key.tunnel.ttl,
+ flower->mask.tunnel.ttl);
 }
 if (flower->key.tunnel.tp_dst) {
 match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst);
@@ -939,6 +941,7 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 const struct flow *key = >flow;
 struct flow *mask = >wc.masks;
 const struct flow_tnl *tnl = >flow.tunnel;
+const struct flow_tnl *tnl_mask = >tunnel;
 struct tc_action *action;
 uint32_t block_id = 0;
 struct nlattr *nla;
@@ -973,6 +976,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.key.tunnel.ttl = tnl->ip_ttl;
 flower.key.tunnel.tp_src = tnl->tp_src;
 flower.key.tunnel.tp_dst = tnl->tp_dst;
+flower.mask.tunnel.tos = tnl_mask->ip_tos;
+flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
 flower.tunnel = true;
 }
 memset(>tunnel, 0, sizeof mask->tunnel);
diff --git a/lib/tc.c b/lib/tc.c
index 22e72ee..52a6697 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -415,13 +415,17 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 flower->key.tunnel.tp_dst =
 nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
 }
-if (attrs[TCA_FLOWER_KEY_ENC_IP_TOS]) {
+if (attrs[TCA_FLOWER_KEY_ENC_IP_TOS_MASK]) {
 flower->key.tunnel.tos =
 nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TOS]);
+flower->mask.tunnel.tos =
+nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TOS_MASK]);
 }
-if (attrs[TCA_FLOWER_KEY_ENC_IP_TTL]) {
+if (attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]) {
 flower->key.tunnel.ttl =
 nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL]);
+flower->mask.tunnel.ttl =
+nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TTL_MASK]);
 }
 }
 
@@ -1623,6 +1627,8 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct 
tc_flower *flower)
 ovs_be32 id = be64_to_be32(flower->key.tunnel.id);
 uint8_t tos = flower->key.tunnel.tos;
 uint8_t ttl = flower->key.tunnel.ttl;
+uint8_t tos_mask = flower->mask.tunnel.tos;
+uint8_t ttl_mask = flower->mask.tunnel.ttl;
 
 if (ipv4_dst) {
 nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_IPV4_SRC, ipv4_src);
@@ -1631,11 +1637,13 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct 
tc_flower *flower)
 nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_SRC, ipv6_src);
 nl_msg_put_in6_addr(request, TCA_FLOWER_KEY_ENC_IPV6_DST, ipv6_dst);
 }
-if (tos) {
+if (tos_mask) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS, tos);
+nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TOS_MASK, tos_mask);
 }
-if (ttl) {
+if (ttl_mask) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
+nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
 }
 nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
 nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
-- 
2.5.5

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

Re: [ovs-dev] [PATCH V2 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-08-01 Thread Or Gerlitz 
On Wed, Aug 1, 2018 at 2:29 PM, Simon Horman  wrote:
> Hi Or,
>
> On 1 August 2018 at 13:21, Or Gerlitz  wrote:
>>
>> On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman 
>> wrote:
>> > On 1 August 2018 at 11:31, Simon Horman 
>> > wrote:
>> >>
>> >> Thanks Or, Thanks Ben,
>> >>
>> >> On 1 August 2018 at 08:43, Or Gerlitz  wrote:
>> >>>
>> >>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz 
>> >>> wrote:
>> >>> > This series comes to address the case to set (encap) and match
>> >>> > (decap)
>> >>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g
>> >>> > due to inherit setup of tunnel port for tos or due to specific OF
>> >>> > rule.
>> >>> >
>> >>> > The series is rebased over Jianbo's patches for QinQ [1]
>> >>>
>> >>> FWIW - note that v2 was actually rebased to the master where Jianbo's
>> >>> work
>> >>> is already applied
>> >>
>> >>
>> >> I have also reviewed these patches, tested that travis-ci is happy with
>> >> everything when applied on top of
>> >> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB
>> >> databases."), which was the most recent
>> >> travis-ci-clean commit in the master branch yesterday, and Netronome
>> >> has
>> >> performed some testing in the lab.
>> >>
>> >> Overall I am happy with these patches and plan to apply them later
>> >> today
>> >> after one final run through travis-ci after rebasing onto the current
>> >> master
>> >> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2
>> >> 3/3]
>> >> ovn-northd: Propagate dynamic addresses to port group address sets."]).
>>
>> > Thanks again Or, I have applied this series to master.
>>
>>
>> Thank you.
>>
>> So how is the stable process @ ovs goes? is that documented, where?
>> e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I
>> ask
>> for stable inclusion?

> The usual procedure, as I understand, is to ask if the maintainer doesn't 
> apply
> the fix to the desired stable branches. I'll take the above as a request to
> apply the patch to branch-2.10.
> Do you want it considered for any other stable branches?

Hi Simon,

Yes, please do apply the ttl fix to 2.10 and if possible, to 2.9 as well since
the bug was introduced there.

Also, it would be good if dfa2ccd "lib/tc: Support matching on ip tos"
would also go to 2.10.
I realized that commit 8f283af "netdev-tc-offloads: Implement netdev
flow put using tc interface"
has blindly set the tos field @ the mask to zero (see mask->nw_tos = 0
in netdev_tc_flow_put)
as if we offloaded that to the TC DP, but we didn't..

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

Re: [ovs-dev] [PATCH V2 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-08-01 Thread Or Gerlitz 
On Wed, Aug 1, 2018 at 2:07 PM, Simon Horman  wrote:
> On 1 August 2018 at 11:31, Simon Horman  wrote:
>>
>> Thanks Or, Thanks Ben,
>>
>> On 1 August 2018 at 08:43, Or Gerlitz  wrote:
>>>
>>> On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz 
>>> wrote:
>>> > This series comes to address the case to set (encap) and match (decap)
>>> > also the tos and ttl fields of TC based IP tunnels. This happens e.g
>>> > due to inherit setup of tunnel port for tos or due to specific OF rule.
>>> >
>>> > The series is rebased over Jianbo's patches for QinQ [1]
>>>
>>> FWIW - note that v2 was actually rebased to the master where Jianbo's
>>> work
>>> is already applied
>>
>>
>> I have also reviewed these patches, tested that travis-ci is happy with
>> everything when applied on top of
>> 185b13f228ac ("ovn: Add Meter and Meter_Band tables to the NB and SB
>> databases."), which was the most recent
>> travis-ci-clean commit in the master branch yesterday, and Netronome has
>> performed some testing in the lab.
>>
>> Overall I am happy with these patches and plan to apply them later today
>> after one final run through travis-ci after rebasing onto the current master
>> branch (which is not travis-ci-clean :( [See: "Re: [ovs-dev] [PATCH v2 3/3]
>> ovn-northd: Propagate dynamic addresses to port group address sets."]).

> Thanks again Or, I have applied this series to master.


Thank you.

So how is the stable process @ ovs goes? is that documented, where?
e.g b4496fc "lib/tc: Handle ttl for ipv6 too" is a bug fix, should/who I ask
for stable inclusion?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH V2 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-08-01 Thread Or Gerlitz 
On Tue, Jul 31, 2018 at 1:40 PM, Or Gerlitz  wrote:
> This series comes to address the case to set (encap) and match (decap)
> also the tos and ttl fields of TC based IP tunnels. This happens e.g
> due to inherit setup of tunnel port for tos or due to specific OF rule.
>
> The series is rebased over Jianbo's patches for QinQ [1]

FWIW - note that v2 was actually rebased to the master where Jianbo's work
is already applied
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH V2 3/4] lib/tc: Support setting tos and ttl for TC IP tunnels

2018-07-31 Thread Or Gerlitz 
Allow to set the tos and ttl for TC tunnels.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 acinclude.m4 |  6 +++---
 include/linux/tc_act/tc_tunnel_key.h | 10 --
 lib/netdev-tc-offloads.c | 16 
 lib/tc.c | 19 +--
 lib/tc.h |  2 ++
 5 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index d6e0d33..a5ff325 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -192,10 +192,10 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
-int x = TCA_TUNNEL_KEY_ENC_DST_PORT;
+int x = TCA_TUNNEL_KEY_ENC_TTL;
 ])],
-[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT], [1],
-   [Define to 1 if TCA_TUNNEL_KEY_ENC_DST_PORT is avaiable.])])
+[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
+   [Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
diff --git a/include/linux/tc_act/tc_tunnel_key.h 
b/include/linux/tc_act/tc_tunnel_key.h
index 0e49834..26cbd2f 100644
--- a/include/linux/tc_act/tc_tunnel_key.h
+++ b/include/linux/tc_act/tc_tunnel_key.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
 #define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
 
-#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT)
+#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
 #include_next 
 #else
 
@@ -36,11 +36,17 @@ enum {
TCA_TUNNEL_KEY_ENC_KEY_ID,  /* be64 */
TCA_TUNNEL_KEY_PAD,
TCA_TUNNEL_KEY_ENC_DST_PORT,/* be16 */
+   TCA_TUNNEL_KEY_NO_CSUM, /* u8 */
+   TCA_TUNNEL_KEY_ENC_OPTS,/* Nested TCA_TUNNEL_KEY_ENC_OPTS_
+ * attributes
+ */
+   TCA_TUNNEL_KEY_ENC_TOS, /* u8 */
+   TCA_TUNNEL_KEY_ENC_TTL, /* u8 */
__TCA_TUNNEL_KEY_MAX,
 };
 
 #define TCA_TUNNEL_KEY_MAX (__TCA_TUNNEL_KEY_MAX - 1)
 
-#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT */
+#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
 
 #endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index c61197a..7ed0844 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -563,6 +563,14 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_in6_addr(buf, OVS_TUNNEL_KEY_ATTR_IPV6_DST,
 >encap.ipv6.ipv6_dst);
 }
+if (action->encap.tos) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TOS,
+  action->encap.tos);
+}
+if (action->encap.ttl) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TTL,
+  action->encap.ttl);
+}
 nl_msg_put_be16(buf, OVS_TUNNEL_KEY_ATTR_TP_DST,
 action->encap.tp_dst);
 
@@ -746,6 +754,14 @@ parse_put_flow_set_action(struct tc_flower *flower, struct 
tc_action *action,
 action->encap.ipv4.ipv4_dst = nl_attr_get_be32(tun_attr);
 }
 break;
+case OVS_TUNNEL_KEY_ATTR_TOS: {
+action->encap.tos = nl_attr_get_u8(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_TTL: {
+action->encap.ttl = nl_attr_get_u8(tun_attr);
+}
+break;
 case OVS_TUNNEL_KEY_ATTR_IPV6_SRC: {
 action->encap.ipv6.ipv6_src =
 nl_attr_get_in6_addr(tun_attr);
diff --git a/lib/tc.c b/lib/tc.c
index ec7efff..d9c9ffa 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -641,6 +641,8 @@ static const struct nl_policy tunnel_key_policy[] = {
   .optional = true, },
 [TCA_TUNNEL_KEY_ENC_KEY_ID] = { .type = NL_A_U32, .optional = true, },
 [TCA_TUNNEL_KEY_ENC_DST_PORT] = { .type = NL_A_U16, .optional = true, },
+[TCA_TUNNEL_KEY_ENC_TOS] = { .type = NL_A_U8, .optional = true, },
+[TCA_TUNNEL_KEY_ENC_TTL] = { .type = NL_A_U8, .optional = true, },
 };
 
 static int
@@ -666,6 +668,8 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 struct nlattr *ipv4_dst = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV4_DST];
 struct nlattr *ipv6_src = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV6_SRC];
 struct nlattr *ipv6_dst = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV6_DST];
+struct nlattr *tos = tun_attrs[TCA_TUNNEL_KEY_ENC_TOS];
+struct nlattr *ttl = tun_attrs[TCA_TUNNEL_KEY_ENC_TTL];
 
 action = >actions[flower->action_count++];
 action->type = TC_ACT_ENCAP;
@@ -679,6 +683,8 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 }

[ovs-dev] [PATCH V2 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-07-31 Thread Or Gerlitz 
This series comes to address the case to set (encap) and match (decap)
also the tos and ttl fields of TC based IP tunnels. This happens e.g
due to inherit setup of tunnel port for tos or due to specific OF rule.

The series is rebased over Jianbo's patches for QinQ [1]

Or.

v2 changes: 
 - rebased to include Jianbo's changes from the master and not locally
 - addressed comment from Simon on duplicated code

Or Gerlitz (4):
  lib/tc: Handle ttl for ipv6 too
  lib/tc: Support matching on ip tos
  lib/tc: Support setting tos and ttl for TC IP tunnels
  lib/tc: Support matching on ip tunnel tos and ttl

 acinclude.m4 | 16 +-
 include/linux/pkt_cls.h  |  7 -
 include/linux/tc_act/tc_tunnel_key.h | 10 +--
 include/openvswitch/match.h  |  1 +
 lib/match.c  |  7 +
 lib/netdev-tc-offloads.c | 34 ++---
 lib/tc.c | 58 +---
 lib/tc.h |  7 -
 8 files changed, 120 insertions(+), 20 deletions(-)

-- 
2.5.5

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

[ovs-dev] [PATCH V2 4/4] lib/tc: Support matching on ip tunnel tos and ttl

2018-07-31 Thread Or Gerlitz 
Support matching on tos and ttl of ip tunnels
for the TC data-path.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 acinclude.m4 | 10 +-
 include/linux/pkt_cls.h  |  7 ++-
 lib/netdev-tc-offloads.c |  8 
 lib/tc.c | 26 +-
 lib/tc.h |  4 +++-
 5 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index a5ff325..1e42599 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -178,17 +178,17 @@ dnl Configure Linux tc compat.
 AC_DEFUN([OVS_CHECK_LINUX_TC], [
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
-int x = TCA_FLOWER_KEY_CVLAN_PRIO;
+int x = TCA_FLOWER_KEY_ENC_IP_TTL_MASK;
 ])],
-[AC_DEFINE([HAVE_TCA_FLOWER_KEY_CVLAN_PRIO], [1],
-   [Define to 1 if TCA_FLOWER_KEY_CVLAN_PRIO is avaiable.])])
+[AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_IP_TTL_MASK], [1],
+   [Define to 1 if TCA_FLOWER_KEY_ENC_IP_TTL_MASK is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
 int x = TCA_VLAN_PUSH_VLAN_PRIORITY;
 ])],
 [AC_DEFINE([HAVE_TCA_VLAN_PUSH_VLAN_PRIORITY], [1],
-   [Define to 1 if TCA_VLAN_PUSH_VLAN_PRIORITY is avaiable.])])
+   [Define to 1 if TCA_VLAN_PUSH_VLAN_PRIORITY is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
@@ -202,7 +202,7 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
 int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
 ])],
 [AC_DEFINE([HAVE_TCA_PEDIT_KEY_EX_HDR_TYPE_UDP], [1],
-   [Define to 1 if TCA_PEDIT_KEY_EX_HDR_TYPE_UDP is avaiable.])])
+   [Define to 1 if TCA_PEDIT_KEY_EX_HDR_TYPE_UDP is available.])])
 ])
 
 dnl OVS_CHECK_DPDK
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 442323d..a330041 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_PKT_CLS_WRAPPER_H
 #define __LINUX_PKT_CLS_WRAPPER_H 1
 
-#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_CVLAN_PRIO)
+#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_IP_TTL_MASK)
 #include_next 
 #else
 
@@ -200,6 +200,11 @@ enum {
TCA_FLOWER_KEY_CVLAN_PRIO,  /* u8   */
TCA_FLOWER_KEY_CVLAN_ETH_TYPE,  /* be16 */
 
+   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TOS_MASK, /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL_MASK, /* u8 */
+
__TCA_FLOWER_MAX,
 };
 
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 7ed0844..7bc745e 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -510,6 +510,12 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_tun_ipv6_src(match, >tunnel.ipv6.ipv6_src);
 match_set_tun_ipv6_dst(match, >tunnel.ipv6.ipv6_dst);
 }
+if (flower->tunnel.tos) {
+match_set_tun_tos(match, flower->tunnel.tos);
+}
+if (flower->tunnel.ttl) {
+match_set_tun_ttl(match, flower->tunnel.ttl);
+}
 if (flower->tunnel.tp_dst) {
 match_set_tun_tp_dst(match, flower->tunnel.tp_dst);
 }
@@ -963,6 +969,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.tunnel.ipv4.ipv4_dst = tnl->ip_dst;
 flower.tunnel.ipv6.ipv6_src = tnl->ipv6_src;
 flower.tunnel.ipv6.ipv6_dst = tnl->ipv6_dst;
+flower.tunnel.tos = tnl->ip_tos;
+flower.tunnel.ttl = tnl->ip_ttl;
 flower.tunnel.tp_src = tnl->tp_src;
 flower.tunnel.tp_dst = tnl->tp_dst;
 flower.tunnel.tunnel = true;
diff --git a/lib/tc.c b/lib/tc.c
index d9c9ffa..bbc3823 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -313,6 +313,14 @@ static const struct nl_policy tca_flower_policy[] = {
 [TCA_FLOWER_KEY_CVLAN_ID] = { .type = NL_A_U16, .optional = true, },
 [TCA_FLOWER_KEY_CVLAN_PRIO] = { .type = NL_A_U8, .optional = true, },
 [TCA_FLOWER_KEY_CVLAN_ETH_TYPE] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_ENC_IP_TOS] = { .type = NL_A_U8,
+.optional = true, },
+[TCA_FLOWER_KEY_ENC_IP_TOS_MASK] = { .type = NL_A_U8,
+ .optional = true, },
+[TCA_FLOWER_KEY_ENC_IP_TTL] = { .type = NL_A_U8,
+.optional = true, },
+[TCA_FLOWER_KEY_ENC_IP_TTL_MASK] = { .type = NL_A_U8,
+ .optional = true, },
 };
 
 static void
@@ -407,6 +415,14 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct 
tc_flower *flower)
 flower->tunnel.tp_dst =
 nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
 }
+if (attrs[TCA_FLOWER_KEY_ENC_IP_TOS]) {
+flower->tunnel.tos =
+nl_attr_get_u8(attrs[TCA_FLOWER_KEY_ENC_IP_TOS]);
+}
+if (

[ovs-dev] [PATCH V2 2/4] lib/tc: Support matching on ip tos

2018-07-31 Thread Or Gerlitz 
Add the missing code to match on ip tos when dealing
with the TC data-path.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 include/openvswitch/match.h |  1 +
 lib/match.c |  7 +++
 lib/netdev-tc-offloads.c|  6 --
 lib/tc.c| 10 ++
 lib/tc.h|  1 +
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index b43ecb1..e8c80dd 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -194,6 +194,7 @@ void match_set_nw_dscp(struct match *, uint8_t);
 void match_set_nw_ecn(struct match *, uint8_t);
 void match_set_nw_ttl(struct match *, uint8_t nw_ttl);
 void match_set_nw_ttl_masked(struct match *, uint8_t nw_ttl, uint8_t mask);
+void match_set_nw_tos_masked(struct match *, uint8_t nw_tos, uint8_t mask);
 void match_set_nw_frag(struct match *, uint8_t nw_frag);
 void match_set_nw_frag_masked(struct match *, uint8_t nw_frag, uint8_t mask);
 void match_set_icmp_type(struct match *, uint8_t);
diff --git a/lib/match.c b/lib/match.c
index 2281fa0..a1407a8 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -946,6 +946,13 @@ match_set_nw_ttl(struct match *match, uint8_t nw_ttl)
 }
 
 void
+match_set_nw_tos_masked(struct match *match, uint8_t nw_tos, uint8_t mask)
+{
+match->flow.nw_tos = nw_tos & mask;
+match->wc.masks.nw_tos = mask;
+}
+
+void
 match_set_nw_ttl_masked(struct match *match, uint8_t nw_ttl, uint8_t mask)
 {
 match->flow.nw_ttl = nw_ttl & mask;
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 2a6dd6d..c61197a 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -455,6 +455,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_nw_proto(match, key->ip_proto);
 }
 
+match_set_nw_tos_masked(match, key->ip_tos, mask->ip_tos);
 match_set_nw_ttl_masked(match, key->ip_ttl, mask->ip_ttl);
 
 if (mask->flags) {
@@ -1026,6 +1027,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.key.ip_proto = key->nw_proto;
 flower.mask.ip_proto = mask->nw_proto;
 mask->nw_proto = 0;
+flower.key.ip_tos = key->nw_tos;
+flower.mask.ip_tos = mask->nw_tos;
+mask->nw_tos = 0;
 flower.key.ip_ttl = key->nw_ttl;
 flower.mask.ip_ttl = mask->nw_ttl;
 mask->nw_ttl = 0;
@@ -1074,8 +1078,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 mask->tp_dst = 0;
 }
 
-mask->nw_tos = 0;
-
 if (key->dl_type == htons(ETH_P_IP)) {
 flower.key.ipv4.ipv4_src = key->nw_src;
 flower.mask.ipv4.ipv4_src = mask->nw_src;
diff --git a/lib/tc.c b/lib/tc.c
index 791f285..ec7efff 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -302,6 +302,10 @@ static const struct nl_policy tca_flower_policy[] = {
 .optional = true, },
 [TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NL_A_U8,
  .optional = true, },
+[TCA_FLOWER_KEY_IP_TOS] = { .type = NL_A_U8,
+.optional = true, },
+[TCA_FLOWER_KEY_IP_TOS_MASK] = { .type = NL_A_U8,
+ .optional = true, },
 [TCA_FLOWER_KEY_TCP_FLAGS] = { .type = NL_A_U16,
.optional = true, },
 [TCA_FLOWER_KEY_TCP_FLAGS_MASK] = { .type = NL_A_U16,
@@ -497,6 +501,11 @@ nl_parse_flower_ip(struct nlattr **attrs, struct tc_flower 
*flower) {
 key->ip_ttl = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TTL]);
 mask->ip_ttl = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TTL_MASK]);
 }
+
+if (attrs[TCA_FLOWER_KEY_IP_TOS_MASK]) {
+key->ip_tos = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TOS]);
+mask->ip_tos = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TOS_MASK]);
+}
 }
 
 static enum tc_offloaded_state
@@ -1626,6 +1635,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 
 if (host_eth_type == ETH_P_IP || host_eth_type == ETH_P_IPV6) {
 FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
+FLOWER_PUT_MASKED_VALUE(ip_tos, TCA_FLOWER_KEY_IP_TOS);
 
 if (flower->mask.ip_proto && flower->key.ip_proto) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_IP_PROTO,
diff --git a/lib/tc.h b/lib/tc.h
index 447d85f..90ef32a 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -95,6 +95,7 @@ struct tc_flower_key {
 
 uint8_t flags;
 uint8_t ip_ttl;
+uint8_t ip_tos;
 
 struct {
 ovs_be32 ipv4_src;
-- 
2.5.5

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

[ovs-dev] [PATCH V2 1/4] lib/tc: Handle ttl for ipv6 too

2018-07-31 Thread Or Gerlitz 
TTL can and should be used to match on IPv6's hop-limit, fix that.

Fixes: ab7ecf266b0a ('netdev-tc-offloads: Add nw_ttl matching using flower')
Fixes: 0b4b5203d12e ('tc: Add ip layer ttl matching')
Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 lib/netdev-tc-offloads.c | 4 ++--
 lib/tc.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 04548c7..2a6dd6d 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -1025,8 +1025,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 if (is_ip_any(key)) {
 flower.key.ip_proto = key->nw_proto;
 flower.mask.ip_proto = mask->nw_proto;
+mask->nw_proto = 0;
 flower.key.ip_ttl = key->nw_ttl;
 flower.mask.ip_ttl = mask->nw_ttl;
+mask->nw_ttl = 0;
 
 if (mask->nw_frag & FLOW_NW_FRAG_ANY) {
 flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
@@ -1073,8 +1075,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 
 mask->nw_tos = 0;
-mask->nw_proto = 0;
-mask->nw_ttl = 0;
 
 if (key->dl_type == htons(ETH_P_IP)) {
 flower.key.ipv4.ipv4_src = key->nw_src;
diff --git a/lib/tc.c b/lib/tc.c
index 22c76b6..791f285 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1625,6 +1625,8 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 FLOWER_PUT_MASKED_VALUE(src_mac, TCA_FLOWER_KEY_ETH_SRC);
 
 if (host_eth_type == ETH_P_IP || host_eth_type == ETH_P_IPV6) {
+FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
+
 if (flower->mask.ip_proto && flower->key.ip_proto) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_IP_PROTO,
   flower->key.ip_proto);
@@ -1653,7 +1655,6 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 if (host_eth_type == ETH_P_IP) {
 FLOWER_PUT_MASKED_VALUE(ipv4.ipv4_src, TCA_FLOWER_KEY_IPV4_SRC);
 FLOWER_PUT_MASKED_VALUE(ipv4.ipv4_dst, TCA_FLOWER_KEY_IPV4_DST);
-FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
 } else if (host_eth_type == ETH_P_IPV6) {
 FLOWER_PUT_MASKED_VALUE(ipv6.ipv6_src, TCA_FLOWER_KEY_IPV6_SRC);
 FLOWER_PUT_MASKED_VALUE(ipv6.ipv6_dst, TCA_FLOWER_KEY_IPV6_DST);
-- 
2.5.5

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

Re: [ovs-dev] [PATCH 4/4] lib/tc: Support matching on ip tunnel tos and ttl

2018-07-26 Thread Or Gerlitz 
On Thu, Jul 26, 2018 at 11:19 PM, Simon Horman
 wrote:
> On Wed, Jul 25, 2018 at 09:20:09PM +0300, Or Gerlitz wrote:

>> +++ b/lib/tc.c
>> @@ -298,6 +298,10 @@ static const struct nl_policy tca_flower_policy[] = {
>>.optional = true, },
>>  [TCA_FLOWER_KEY_FLAGS] = { .type = NL_A_BE32, .optional = true, },
>>  [TCA_FLOWER_KEY_FLAGS_MASK] = { .type = NL_A_BE32, .optional = true, },
>> +[TCA_FLOWER_KEY_TCP_FLAGS] = { .type = NL_A_U16,
>> +   .optional = true, },
>> +[TCA_FLOWER_KEY_TCP_FLAGS_MASK] = { .type = NL_A_U16,
>> +.optional = true, },


> The above hunk appears to duplicate existing initialisation of the same
> indexes of the array. Perhaps it should be dropped?

sure, I will fix it up, thanks for the catch!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/4] lib/tc: Handle ttl for ipv6 too

2018-07-26 Thread Or Gerlitz 
On Thu, Jul 26, 2018 at 5:41 PM, Simon Horman
 wrote:
> On Wed, Jul 25, 2018 at 09:20:06PM +0300, Or Gerlitz wrote:
>> TTL can and should be used to match on IPv6's hop-limit, fix that.

>> Fixes: ab7ecf266b0a ('netdev-tc-offloads: Add nw_ttl matching using flower')
>> Fixes: 0b4b5203d12e ('tc: Add ip layer ttl matching')

>> --- a/lib/netdev-tc-offloads.c
>> +++ b/lib/netdev-tc-offloads.c
>> @@ -1025,8 +1025,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct 
>> match *match,
>>  if (is_ip_any(key)) {
>>  flower.key.ip_proto = key->nw_proto;
>>  flower.mask.ip_proto = mask->nw_proto;
>> +mask->nw_proto = 0;
>>  flower.key.ip_ttl = key->nw_ttl;
>>  flower.mask.ip_ttl = mask->nw_ttl;
>> +mask->nw_ttl = 0;
>>
>>  if (mask->nw_frag & FLOW_NW_FRAG_ANY) {
>>  flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
>> @@ -1073,8 +1075,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
>> *match,
>>  }
>>
>>  mask->nw_tos = 0;
>> -mask->nw_proto = 0;
>> -mask->nw_ttl = 0;
>
> I'm not sure that I understand the purpose of the changes above.
> They seem to shuffle setting two mask values from one place to another.
> But what is the effect of this?

Setting mask->zzz to 0 means we consumed (== set into the mask
of the tc rule) the zzz field. The convention in the code is to have
this zeroing near the spot where you consume the field, I aligned
this code to that convention while fixing the bug.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/4] lib/tc: Support matching on ip tos

2018-07-26 Thread Or Gerlitz 
On Thu, Jul 26, 2018 at 5:44 PM, Simon Horman
 wrote:
> On Wed, Jul 25, 2018 at 09:20:07PM +0300, Or Gerlitz wrote:
>> Add the missing code to match on ip tos when dealing
>> with the TC data-path.

>> --- a/lib/netdev-tc-offloads.c
>> +++ b/lib/netdev-tc-offloads.c
>> @@ -455,6 +455,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
>>  match_set_nw_proto(match, key->ip_proto);
>>  }
>>
>> +match_set_nw_tos_masked(match, key->ip_tos, mask->ip_tos);
>>  match_set_nw_ttl_masked(match, key->ip_ttl, mask->ip_ttl);
>>
>>  if (mask->flags) {
>> @@ -1026,6 +1027,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
>> *match,
>>  flower.key.ip_proto = key->nw_proto;
>>  flower.mask.ip_proto = mask->nw_proto;
>>  mask->nw_proto = 0;
>> +flower.key.ip_tos = key->nw_tos;
>> +flower.mask.ip_tos = mask->nw_tos;
>> +mask->nw_tos = 0;
>>  flower.key.ip_ttl = key->nw_ttl;
>>  flower.mask.ip_ttl = mask->nw_ttl;
>>  mask->nw_ttl = 0;
>> @@ -1074,8 +1078,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
>> *match,
>>  mask->tp_dst = 0;
>>  }
>>
>> -mask->nw_tos = 0;

> As per my comment on patch 1/2, the intention of the mask->nw_tos change
> in the above two hunks is not clear to me.

setting mask->zzz to 0 means we consumed (== set into the mask
of the tc rule) the zzz field. Here, the field was marked as consumed but it
wasn't such. While adding the support to actually use tos, I moved the
zering to be near the usage, to align with the rest of the code.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 4/4] lib/tc: Support matching on ip tunnel tos and ttl

2018-07-25 Thread Or Gerlitz 
Support matching on tos and ttl of ip tunnels
for the TC data-path.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 acinclude.m4 | 10 +-
 include/linux/pkt_cls.h  |  7 ++-
 lib/netdev-tc-offloads.c |  8 
 lib/tc.c | 30 +-
 lib/tc.h |  4 +++-
 5 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index a5ff325..1e42599 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -178,17 +178,17 @@ dnl Configure Linux tc compat.
 AC_DEFUN([OVS_CHECK_LINUX_TC], [
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
-int x = TCA_FLOWER_KEY_CVLAN_PRIO;
+int x = TCA_FLOWER_KEY_ENC_IP_TTL_MASK;
 ])],
-[AC_DEFINE([HAVE_TCA_FLOWER_KEY_CVLAN_PRIO], [1],
-   [Define to 1 if TCA_FLOWER_KEY_CVLAN_PRIO is avaiable.])])
+[AC_DEFINE([HAVE_TCA_FLOWER_KEY_ENC_IP_TTL_MASK], [1],
+   [Define to 1 if TCA_FLOWER_KEY_ENC_IP_TTL_MASK is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
 int x = TCA_VLAN_PUSH_VLAN_PRIORITY;
 ])],
 [AC_DEFINE([HAVE_TCA_VLAN_PUSH_VLAN_PRIORITY], [1],
-   [Define to 1 if TCA_VLAN_PUSH_VLAN_PRIORITY is avaiable.])])
+   [Define to 1 if TCA_VLAN_PUSH_VLAN_PRIORITY is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
@@ -202,7 +202,7 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
 int x = TCA_PEDIT_KEY_EX_HDR_TYPE_UDP;
 ])],
 [AC_DEFINE([HAVE_TCA_PEDIT_KEY_EX_HDR_TYPE_UDP], [1],
-   [Define to 1 if TCA_PEDIT_KEY_EX_HDR_TYPE_UDP is avaiable.])])
+   [Define to 1 if TCA_PEDIT_KEY_EX_HDR_TYPE_UDP is available.])])
 ])
 
 dnl OVS_CHECK_DPDK
diff --git a/include/linux/pkt_cls.h b/include/linux/pkt_cls.h
index 442323d..a330041 100644
--- a/include/linux/pkt_cls.h
+++ b/include/linux/pkt_cls.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_PKT_CLS_WRAPPER_H
 #define __LINUX_PKT_CLS_WRAPPER_H 1
 
-#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_CVLAN_PRIO)
+#if defined(__KERNEL__) || defined(HAVE_TCA_FLOWER_KEY_ENC_IP_TTL_MASK)
 #include_next 
 #else
 
@@ -200,6 +200,11 @@ enum {
TCA_FLOWER_KEY_CVLAN_PRIO,  /* u8   */
TCA_FLOWER_KEY_CVLAN_ETH_TYPE,  /* be16 */
 
+   TCA_FLOWER_KEY_ENC_IP_TOS,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TOS_MASK, /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL,  /* u8 */
+   TCA_FLOWER_KEY_ENC_IP_TTL_MASK, /* u8 */
+
__TCA_FLOWER_MAX,
 };
 
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 7ed0844..7bc745e 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -510,6 +510,12 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_tun_ipv6_src(match, >tunnel.ipv6.ipv6_src);
 match_set_tun_ipv6_dst(match, >tunnel.ipv6.ipv6_dst);
 }
+if (flower->tunnel.tos) {
+match_set_tun_tos(match, flower->tunnel.tos);
+}
+if (flower->tunnel.ttl) {
+match_set_tun_ttl(match, flower->tunnel.ttl);
+}
 if (flower->tunnel.tp_dst) {
 match_set_tun_tp_dst(match, flower->tunnel.tp_dst);
 }
@@ -963,6 +969,8 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.tunnel.ipv4.ipv4_dst = tnl->ip_dst;
 flower.tunnel.ipv6.ipv6_src = tnl->ipv6_src;
 flower.tunnel.ipv6.ipv6_dst = tnl->ipv6_dst;
+flower.tunnel.tos = tnl->ip_tos;
+flower.tunnel.ttl = tnl->ip_ttl;
 flower.tunnel.tp_src = tnl->tp_src;
 flower.tunnel.tp_dst = tnl->tp_dst;
 flower.tunnel.tunnel = true;
diff --git a/lib/tc.c b/lib/tc.c
index aa3147f..f5901d1 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -298,6 +298,10 @@ static const struct nl_policy tca_flower_policy[] = {
   .optional = true, },
 [TCA_FLOWER_KEY_FLAGS] = { .type = NL_A_BE32, .optional = true, },
 [TCA_FLOWER_KEY_FLAGS_MASK] = { .type = NL_A_BE32, .optional = true, },
+[TCA_FLOWER_KEY_TCP_FLAGS] = { .type = NL_A_U16,
+   .optional = true, },
+[TCA_FLOWER_KEY_TCP_FLAGS_MASK] = { .type = NL_A_U16,
+.optional = true, },
 [TCA_FLOWER_KEY_IP_TTL] = { .type = NL_A_U8,
 .optional = true, },
 [TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NL_A_U8,
@@ -313,6 +317,14 @@ static const struct nl_policy tca_flower_policy[] = {
 [TCA_FLOWER_KEY_CVLAN_ID] = { .type = NL_A_U16, .optional = true, },
 [TCA_FLOWER_KEY_CVLAN_PRIO] = { .type = NL_A_U8, .optional = true, },
 [TCA_FLOWER_KEY_CVLAN_ETH_TYPE] = { .type = NL_A_U16, .optional = true, },
+[TCA_FLOWER_KEY_ENC_IP_TOS] = { .type = NL_A_U8,
+.optional = true, },
+[TCA_FL

[ovs-dev] [PATCH 3/4] lib/tc: Support setting tos and ttl for TC IP tunnels

2018-07-25 Thread Or Gerlitz 
Allow to set the tos and ttl for TC tunnels.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 acinclude.m4 |  6 +++---
 include/linux/tc_act/tc_tunnel_key.h | 10 --
 lib/netdev-tc-offloads.c | 16 
 lib/tc.c | 19 +--
 lib/tc.h |  2 ++
 5 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index d6e0d33..a5ff325 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -192,10 +192,10 @@ AC_DEFUN([OVS_CHECK_LINUX_TC], [
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
-int x = TCA_TUNNEL_KEY_ENC_DST_PORT;
+int x = TCA_TUNNEL_KEY_ENC_TTL;
 ])],
-[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT], [1],
-   [Define to 1 if TCA_TUNNEL_KEY_ENC_DST_PORT is avaiable.])])
+[AC_DEFINE([HAVE_TCA_TUNNEL_KEY_ENC_TTL], [1],
+   [Define to 1 if TCA_TUNNEL_KEY_ENC_TTL is available.])])
 
   AC_COMPILE_IFELSE([
 AC_LANG_PROGRAM([#include ], [
diff --git a/include/linux/tc_act/tc_tunnel_key.h 
b/include/linux/tc_act/tc_tunnel_key.h
index 0e49834..26cbd2f 100644
--- a/include/linux/tc_act/tc_tunnel_key.h
+++ b/include/linux/tc_act/tc_tunnel_key.h
@@ -1,7 +1,7 @@
 #ifndef __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H
 #define __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H 1
 
-#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT)
+#if defined(__KERNEL__) || defined(HAVE_TCA_TUNNEL_KEY_ENC_TTL)
 #include_next 
 #else
 
@@ -36,11 +36,17 @@ enum {
TCA_TUNNEL_KEY_ENC_KEY_ID,  /* be64 */
TCA_TUNNEL_KEY_PAD,
TCA_TUNNEL_KEY_ENC_DST_PORT,/* be16 */
+   TCA_TUNNEL_KEY_NO_CSUM, /* u8 */
+   TCA_TUNNEL_KEY_ENC_OPTS,/* Nested TCA_TUNNEL_KEY_ENC_OPTS_
+ * attributes
+ */
+   TCA_TUNNEL_KEY_ENC_TOS, /* u8 */
+   TCA_TUNNEL_KEY_ENC_TTL, /* u8 */
__TCA_TUNNEL_KEY_MAX,
 };
 
 #define TCA_TUNNEL_KEY_MAX (__TCA_TUNNEL_KEY_MAX - 1)
 
-#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_DST_PORT */
+#endif /* __KERNEL__ || HAVE_TCA_TUNNEL_KEY_ENC_TTL */
 
 #endif /* __LINUX_TC_ACT_TC_TUNNEL_KEY_WRAPPER_H */
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index c61197a..7ed0844 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -563,6 +563,14 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 nl_msg_put_in6_addr(buf, OVS_TUNNEL_KEY_ATTR_IPV6_DST,
 >encap.ipv6.ipv6_dst);
 }
+if (action->encap.tos) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TOS,
+  action->encap.tos);
+}
+if (action->encap.ttl) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TTL,
+  action->encap.ttl);
+}
 nl_msg_put_be16(buf, OVS_TUNNEL_KEY_ATTR_TP_DST,
 action->encap.tp_dst);
 
@@ -746,6 +754,14 @@ parse_put_flow_set_action(struct tc_flower *flower, struct 
tc_action *action,
 action->encap.ipv4.ipv4_dst = nl_attr_get_be32(tun_attr);
 }
 break;
+case OVS_TUNNEL_KEY_ATTR_TOS: {
+action->encap.tos = nl_attr_get_u8(tun_attr);
+}
+break;
+case OVS_TUNNEL_KEY_ATTR_TTL: {
+action->encap.ttl = nl_attr_get_u8(tun_attr);
+}
+break;
 case OVS_TUNNEL_KEY_ATTR_IPV6_SRC: {
 action->encap.ipv6.ipv6_src =
 nl_attr_get_in6_addr(tun_attr);
diff --git a/lib/tc.c b/lib/tc.c
index 8fbca7d..aa3147f 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -641,6 +641,8 @@ static const struct nl_policy tunnel_key_policy[] = {
   .optional = true, },
 [TCA_TUNNEL_KEY_ENC_KEY_ID] = { .type = NL_A_U32, .optional = true, },
 [TCA_TUNNEL_KEY_ENC_DST_PORT] = { .type = NL_A_U16, .optional = true, },
+[TCA_TUNNEL_KEY_ENC_TOS] = { .type = NL_A_U8, .optional = true, },
+[TCA_TUNNEL_KEY_ENC_TTL] = { .type = NL_A_U8, .optional = true, },
 };
 
 static int
@@ -666,6 +668,8 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 struct nlattr *ipv4_dst = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV4_DST];
 struct nlattr *ipv6_src = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV6_SRC];
 struct nlattr *ipv6_dst = tun_attrs[TCA_TUNNEL_KEY_ENC_IPV6_DST];
+struct nlattr *tos = tun_attrs[TCA_TUNNEL_KEY_ENC_TOS];
+struct nlattr *ttl = tun_attrs[TCA_TUNNEL_KEY_ENC_TTL];
 
 action = >actions[flower->action_count++];
 action->type = TC_ACT_ENCAP;
@@ -679,6 +683,8 @@ nl_parse_act_tunnel_key(struct nlattr *options, struct 
tc_flower *flower)
 }

[ovs-dev] [PATCH 2/4] lib/tc: Support matching on ip tos

2018-07-25 Thread Or Gerlitz 
Add the missing code to match on ip tos when dealing
with the TC data-path.

Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 include/openvswitch/match.h |  1 +
 lib/match.c |  7 +++
 lib/netdev-tc-offloads.c|  6 --
 lib/tc.c| 10 ++
 lib/tc.h|  1 +
 5 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index b43ecb1..e8c80dd 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -194,6 +194,7 @@ void match_set_nw_dscp(struct match *, uint8_t);
 void match_set_nw_ecn(struct match *, uint8_t);
 void match_set_nw_ttl(struct match *, uint8_t nw_ttl);
 void match_set_nw_ttl_masked(struct match *, uint8_t nw_ttl, uint8_t mask);
+void match_set_nw_tos_masked(struct match *, uint8_t nw_tos, uint8_t mask);
 void match_set_nw_frag(struct match *, uint8_t nw_frag);
 void match_set_nw_frag_masked(struct match *, uint8_t nw_frag, uint8_t mask);
 void match_set_icmp_type(struct match *, uint8_t);
diff --git a/lib/match.c b/lib/match.c
index 2281fa0..a1407a8 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -946,6 +946,13 @@ match_set_nw_ttl(struct match *match, uint8_t nw_ttl)
 }
 
 void
+match_set_nw_tos_masked(struct match *match, uint8_t nw_tos, uint8_t mask)
+{
+match->flow.nw_tos = nw_tos & mask;
+match->wc.masks.nw_tos = mask;
+}
+
+void
 match_set_nw_ttl_masked(struct match *match, uint8_t nw_ttl, uint8_t mask)
 {
 match->flow.nw_ttl = nw_ttl & mask;
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 2a6dd6d..c61197a 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -455,6 +455,7 @@ parse_tc_flower_to_match(struct tc_flower *flower,
 match_set_nw_proto(match, key->ip_proto);
 }
 
+match_set_nw_tos_masked(match, key->ip_tos, mask->ip_tos);
 match_set_nw_ttl_masked(match, key->ip_ttl, mask->ip_ttl);
 
 if (mask->flags) {
@@ -1026,6 +1027,9 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 flower.key.ip_proto = key->nw_proto;
 flower.mask.ip_proto = mask->nw_proto;
 mask->nw_proto = 0;
+flower.key.ip_tos = key->nw_tos;
+flower.mask.ip_tos = mask->nw_tos;
+mask->nw_tos = 0;
 flower.key.ip_ttl = key->nw_ttl;
 flower.mask.ip_ttl = mask->nw_ttl;
 mask->nw_ttl = 0;
@@ -1074,8 +1078,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 mask->tp_dst = 0;
 }
 
-mask->nw_tos = 0;
-
 if (key->dl_type == htons(ETH_P_IP)) {
 flower.key.ipv4.ipv4_src = key->nw_src;
 flower.mask.ipv4.ipv4_src = mask->nw_src;
diff --git a/lib/tc.c b/lib/tc.c
index f3fb59c..8fbca7d 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -302,6 +302,10 @@ static const struct nl_policy tca_flower_policy[] = {
 .optional = true, },
 [TCA_FLOWER_KEY_IP_TTL_MASK] = { .type = NL_A_U8,
  .optional = true, },
+[TCA_FLOWER_KEY_IP_TOS] = { .type = NL_A_U8,
+.optional = true, },
+[TCA_FLOWER_KEY_IP_TOS_MASK] = { .type = NL_A_U8,
+ .optional = true, },
 [TCA_FLOWER_KEY_TCP_FLAGS] = { .type = NL_A_U16,
.optional = true, },
 [TCA_FLOWER_KEY_TCP_FLAGS_MASK] = { .type = NL_A_U16,
@@ -497,6 +501,11 @@ nl_parse_flower_ip(struct nlattr **attrs, struct tc_flower 
*flower) {
 key->ip_ttl = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TTL]);
 mask->ip_ttl = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TTL_MASK]);
 }
+
+if (attrs[TCA_FLOWER_KEY_IP_TOS_MASK]) {
+key->ip_tos = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TOS]);
+mask->ip_tos = nl_attr_get_u8(attrs[TCA_FLOWER_KEY_IP_TOS_MASK]);
+}
 }
 
 static enum tc_offloaded_state
@@ -1626,6 +1635,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 
 if (host_eth_type == ETH_P_IP || host_eth_type == ETH_P_IPV6) {
 FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
+FLOWER_PUT_MASKED_VALUE(ip_tos, TCA_FLOWER_KEY_IP_TOS);
 
 if (flower->mask.ip_proto && flower->key.ip_proto) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_IP_PROTO,
diff --git a/lib/tc.h b/lib/tc.h
index 447d85f..90ef32a 100644
--- a/lib/tc.h
+++ b/lib/tc.h
@@ -95,6 +95,7 @@ struct tc_flower_key {
 
 uint8_t flags;
 uint8_t ip_ttl;
+uint8_t ip_tos;
 
 struct {
 ovs_be32 ipv4_src;
-- 
2.5.5

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

[ovs-dev] [PATCH 0/4] Enable set/match of tos/ttl for IP tunnels on TC data-path

2018-07-25 Thread Or Gerlitz 
This series comes to address the case to set (encap) and match (decap)
also the tos and ttl fields of TC based IP tunnels. This happens e.g
due to inherit setup of tunnel port for tos or due to specific OF rule.

The series is rebased over Jianbo's patches for QinQ [1]

Or.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/349752.html

Or Gerlitz (4):
  lib/tc: Handle ttl for ipv6 too
  lib/tc: Support matching on ip tos
  lib/tc: Support setting tos and ttl for TC IP tunnels
  lib/tc: Support matching on ip tunnel tos and ttl

 acinclude.m4 | 16 +-
 include/linux/pkt_cls.h  |  7 +++-
 include/linux/tc_act/tc_tunnel_key.h | 10 --
 include/openvswitch/match.h  |  1 +
 lib/match.c  |  7 
 lib/netdev-tc-offloads.c | 34 +---
 lib/tc.c | 62 +---
 lib/tc.h |  7 +++-
 8 files changed, 124 insertions(+), 20 deletions(-)

-- 
2.5.5

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

[ovs-dev] [PATCH 1/4] lib/tc: Handle ttl for ipv6 too

2018-07-25 Thread Or Gerlitz 
TTL can and should be used to match on IPv6's hop-limit, fix that.

Fixes: ab7ecf266b0a ('netdev-tc-offloads: Add nw_ttl matching using flower')
Fixes: 0b4b5203d12e ('tc: Add ip layer ttl matching')
Signed-off-by: Or Gerlitz 
Reviewed-by: Roi Dayan 
---
 lib/netdev-tc-offloads.c | 4 ++--
 lib/tc.c | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
index 04548c7..2a6dd6d 100644
--- a/lib/netdev-tc-offloads.c
+++ b/lib/netdev-tc-offloads.c
@@ -1025,8 +1025,10 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 if (is_ip_any(key)) {
 flower.key.ip_proto = key->nw_proto;
 flower.mask.ip_proto = mask->nw_proto;
+mask->nw_proto = 0;
 flower.key.ip_ttl = key->nw_ttl;
 flower.mask.ip_ttl = mask->nw_ttl;
+mask->nw_ttl = 0;
 
 if (mask->nw_frag & FLOW_NW_FRAG_ANY) {
 flower.mask.flags |= TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT;
@@ -1073,8 +1075,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match 
*match,
 }
 
 mask->nw_tos = 0;
-mask->nw_proto = 0;
-mask->nw_ttl = 0;
 
 if (key->dl_type == htons(ETH_P_IP)) {
 flower.key.ipv4.ipv4_src = key->nw_src;
diff --git a/lib/tc.c b/lib/tc.c
index 2157135..f3fb59c 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -1625,6 +1625,8 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 FLOWER_PUT_MASKED_VALUE(src_mac, TCA_FLOWER_KEY_ETH_SRC);
 
 if (host_eth_type == ETH_P_IP || host_eth_type == ETH_P_IPV6) {
+FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
+
 if (flower->mask.ip_proto && flower->key.ip_proto) {
 nl_msg_put_u8(request, TCA_FLOWER_KEY_IP_PROTO,
   flower->key.ip_proto);
@@ -1653,7 +1655,6 @@ nl_msg_put_flower_options(struct ofpbuf *request, struct 
tc_flower *flower)
 if (host_eth_type == ETH_P_IP) {
 FLOWER_PUT_MASKED_VALUE(ipv4.ipv4_src, TCA_FLOWER_KEY_IPV4_SRC);
 FLOWER_PUT_MASKED_VALUE(ipv4.ipv4_dst, TCA_FLOWER_KEY_IPV4_DST);
-FLOWER_PUT_MASKED_VALUE(ip_ttl, TCA_FLOWER_KEY_IP_TTL);
 } else if (host_eth_type == ETH_P_IPV6) {
 FLOWER_PUT_MASKED_VALUE(ipv6.ipv6_src, TCA_FLOWER_KEY_IPV6_SRC);
 FLOWER_PUT_MASKED_VALUE(ipv6.ipv6_dst, TCA_FLOWER_KEY_IPV6_DST);
-- 
2.5.5

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

Re: [ovs-dev] [PATCH] tc: Correct convert ticks to msecs on parsing tc TM

2017-07-27 Thread Or Gerlitz 

On 7/27/2017 1:14 PM, Roi Dayan wrote:
the system call is done only once. 


good to know, would be worth to mention that on the change-log, so it's 
clear we're good w.r.t performance.



Or.

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

Re: [ovs-dev] [PATCH] tc: Correct convert ticks to msecs on parsing tc TM

2017-07-27 Thread Or Gerlitz 

On 7/27/2017 11:32 AM, Simon Horman wrote:

On Thu, Jul 27, 2017 at 11:29:10AM +0300, Or Gerlitz wrote:

On 7/27/2017 11:00 AM, Simon Horman wrote:

Use sysconf(_SC_CLK_TCK) to read run time "number of clock ticks per
second" and use that to convert ticks to msecs.
This is how iproute does the conversion when parsing tc filters.

Signed-off-by: Paul Blakey<pa...@mellanox.com>
Reviewed-by: Roi Dayan<r...@mellanox.com>

This looks good to me

Is this/similar conversion is needed also for the non TC data-path? if yes,
how they do it there? if not, can you elaborate who does it (the ovs kernel
dp?)

Hi Or,

the Linux kernel datapath provides an OVS_FLOW_ATTR_USED attribute
whose value is in ms - converted from jiffies in the kernel using
ovs_flow_used_time(). So I don't think the conversion needs
to be done in user-space for that datapath.


I see, so if this patch does it for each dump of each flow, we might 
added (say) 100K system calls per second? doesn't sound cool to me... I 
guess we either need to make sure the system call is done very rarely or 
we can maybe add some flag to tc to get the dump in the units we need.  
Hopefully the 1st option (doing the system call rarely) is viable and we 
can go that path and avoid kernel UAPI changes and patching.


Or.

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

Re: [ovs-dev] [PATCH] tc: Correct convert ticks to msecs on parsing tc TM

2017-07-27 Thread Or Gerlitz 

On 7/27/2017 11:00 AM, Simon Horman wrote:

Use sysconf(_SC_CLK_TCK) to read run time "number of clock ticks per
second" and use that to convert ticks to msecs.
This is how iproute does the conversion when parsing tc filters.

Signed-off-by: Paul Blakey
Reviewed-by: Roi Dayan

This looks good to me


Is this/similar conversion is needed also for the non TC data-path? if 
yes, how they do it there? if not, can you elaborate who does it (the 
ovs kernel dp?)


Or.

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

Re: [ovs-dev] [PATCH] tc: Add SCTP support

2017-07-25 Thread Or Gerlitz 

On 7/25/2017 8:29 AM, Roi Dayan wrote:

Imlement SCTP source and destination ports support for flower


s/Imlement/Implement also would be good to add a period @ the end of the 
sentence.


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

Re: [ovs-dev] [PATCH ovs V4 00/24] Introducing HW offload support for openvswitch

2017-03-20 Thread Or Gerlitz 

On 3/17/2017 4:14 PM, Simon Horman wrote:

Hi again,

I think that once the TODO items above are resolved - both of which I have 
provided some feedback on separately - some consideration should be given to 
merging this patchset.


Any thoughts/comments on that? Roi just commented that he will roll V5 
with few more small fixes, but we'd be happy to hear if otherwise no 
more changes are needed?


Or.



 From my point of view it introduces important features for those interested in 
offloading OvS using the Linux kernel while having minimal impact on those who 
are not. I think it provides a good basis for further work on such offloads. 
And as we are some distance away from the next release we should have plenty of 
time to iron any problems that may be inadvertently introduced by this patchset.



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

Re: [ovs-dev] [PATCH ovs V1 0/9] Introducing HW offload support for openvswitch

2016-11-03 Thread Or Gerlitz 

On 11/3/2016 3:19 PM, Simon Horman wrote:

  Am I right in thinking that the current implementation only allows offloading 
flows that output to one port? If so I'd like to hear your thoughts on how that 
restriction might be lifted.


We are using the TC mirred action. For forwarding to multiple ports the 
code can be enhanced to have multiple mirred actions on that filter, sure.


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH ovs RFC 9/9] netdev-linux: add tc ingress qdisc support

2016-10-05 Thread Or Gerlitz 

On 10/5/2016 3:27 AM, Ben Pfaff wrote:

On Tue, Sep 27, 2016 at 03:46:04PM +0300, Paul Blakey wrote:

>Add tc ingress qdisc support so we can add qdisc
>as a qos on port or through config.
>usage:
>ovs-vsctl -- set port  qos=@newq -- --id=@newq create \
>qos type=linux-ingress
>where  is a already added port using vsctl add-port.
>
>Signed-off-by: Paul Blakey
>Signed-off-by: Shahar Klein

I don't plan to review all of these patches but this one caught my eye.


any specail reason for not looking on this series?

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH/RFC 00/12] Programming Open vSwitch (-like) flows into hardware using SwitchDev

2016-10-01 Thread Or Gerlitz 
On Sat, Oct 1, 2016 at 1:12 AM, pravin shelar  wrote:

[...]

> Why not allow switchdev offload API for userspace similar to TC flower
> offload? or we could use flower API for switchdev flow offload.

Hi Pravin,

Could you also share your thoughts on the RFC we've posted couple of
days ago to the OVS devel mailing list? the offloading there [1] is
from OVSD using TC/Flower

Or.

[1] http://openvswitch.org/pipermail/dev/2016-September/079952.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH/RFC 00/12] Programming Open vSwitch (-like) flows into hardware using SwitchDev

2016-09-28 Thread Or Gerlitz 
On Wed, Sep 28, 2016 at 3:42 PM, Simon Horman
 wrote:

> A different approach, not implemented by this patch-set, is for user-space
> to program flows into hardware by some other means, for example TC, and/or
> the (kernel) datapath.

Right, and we've submitted that code to the OVS community 24h ago [1].

This was done along the feedback we've got for the last two years (since
the  LPC 2014 networking micro-conf). It allows offloading from
multiple user-space
applications through a single UAPI -- the TC one (currently we did
flower, but the OVSD
patch set can be extended to use whatever TC offloads are supported by
the port driver,
e.g U32, eBPF) and integration with 3rd party policy modules  running
in user-space.

Lets hear people opinions and see where we go from now.

> I believe that approach does not conflict with this one.
>  And there is some scope to share infrastructure in the kernel

maybe, possibly

We've having a talk in netdev 1.2 on offloading HW offloading of OVS
and similar applications,
I would encourage people to come and approach me and/or Rony Efraim
from Mellanox before/after
the talk to discuss that F2F, would love to get feedbacks, and also here...

Or.

[1] pointers to patches implementing the 2nd approach

cover-letter http://openvswitch.org/pipermail/dev/2016-September/079952.html

patches

https://patchwork.ozlabs.org/patch/675560/
https://patchwork.ozlabs.org/patch/675567/
https://patchwork.ozlabs.org/patch/675565/
https://patchwork.ozlabs.org/patch/675559/
https://patchwork.ozlabs.org/patch/675564/
https://patchwork.ozlabs.org/patch/675563/
https://patchwork.ozlabs.org/patch/675568/
https://patchwork.ozlabs.org/patch/675566/
https://patchwork.ozlabs.org/patch/675562/

[2] http://www.netdevconf.org/1.2/session.html?rony-efraim-1
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH nf-next v8 1/8] netfilter: Remove IP_CT_NEW_REPLY definition.

2016-03-10 Thread Or Gerlitz 
On Wed, Mar 9, 2016 at 2:24 AM, Jarno Rajahalme  wrote:
> Remove the definition of IP_CT_NEW_REPLY from the kernel as it does
> not make sense.  This allows the definition of IP_CT_NUMBER to be
> simplified as well.

I just realized that after V7 you stopped sending cover letter (patch 0/N)
with this series.

Maybe you send it and this misses the list? we need to be able to see
differences from earlier versions.

Or.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [patch net-next v2 8/9] switchdev: introduce Netlink API

2014-10-06 Thread Or Gerlitz 
On Sat, Sep 27, 2014 at 12:02 AM, Thomas Graf tg...@suug.ch wrote:
 On 09/26/14 at 11:03pm, Or Gerlitz wrote:
 Yep, this can serve us for the architecture discussion @ LPC. Re the
 SRIOV case, you referred to the case where guest VF traffic goes
 through HW (say) VXLAN encap/decap -- just to make sure, we need also
 to support the simpler case, where guest traffic just goes through
 vlan tag/strip.

 Agreed.

 The SRIOV case is only mentioned here in the Compatibility with
 existing FDB ioctls for SR-IOV bullet, so I'm a bit nervous... we
 need to have it clear in the agenda.

 I think the offload API discussion should consider the SR-iOV case
 but we might need to discuss additional details outside of that
 BoF to ensure that the BoF can keep focus on the offload API itself.
 That said, I suggest we define the specific agenda once we know that
 the BoF has been accepted and 2 hours have been allocated ;-)

 Also, this BoF needs to be double-len, two hours, can you act to
 get that done?

 This has already been requested.

Is this confirmed?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [patch net-next v2 8/9] switchdev: introduce Netlink API

2014-09-26 Thread Or Gerlitz 
On Wed, Sep 24, 2014 at 4:32 PM, Thomas Graf tg...@suug.ch wrote:
 On 09/23/14 at 06:32pm, Or Gerlitz wrote:
 Indeed.

 The idea is to leverage OVS to manage eSwitch (embedded NIC switch) as well
 (NOT to offload OVS).

 We envision a seamless integration of user environment which is based on OVS
 with SRIOV eSwitch and the grounds for that were very well supported in
 Jiri’s V1.

 Please consider comparing your model with what is described here [0].
 I'm trying to write down an architecture document that we can finalize
 in Düsseldorf.
 [0] http://goo.gl/qkzW5y

Yep, this can serve us for the architecture discussion @ LPC. Re the
SRIOV case, you referred to the case where guest VF traffic goes
through HW (say) VXLAN encap/decap -- just to make sure, we need also
to support the simpler case, where guest traffic just goes through
vlan tag/strip.


 The eSwitch hardware does not need to have multiple tables and ‘enjoys’ the
 flat rule of OVS. The kernel datapath does not need to be aware of the
 existence of HW nor its capabilities, it just pushes the flow also to the
 switchdev which represents the eSwitch.

 I think you are saying that the kernel should not be required to make
 the offload decision which is fair. We definitely don't want to force
 the decision to be outside though, there are several legit reasons to
 support transparent offloads within the kernel as well outside of OVS.

 Yep. LPC is the time and place to go over the multiple use-cases (phyiscal
 switch, eSwitch, eBPF, etc) that could (should) be supported by the basic
 framework.

 For reference:
 http://www.linuxplumbersconf.org/2014/ocw/proposals/2463

The SRIOV case is only mentioned here in the Compatibility with
existing FDB ioctls for SR-IOV bullet, so I'm a bit nervous... we
need to have it clear in the agenda. Also, this BoF needs to be
double-len, two hours, can you act to get that done?

thanks,

Or.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [patch net-next v2 8/9] switchdev: introduce Netlink API

2014-09-23 Thread Or Gerlitz 


On 9/23/2014 7:11 AM, Andy Gospodarek wrote:

On Mon, Sep 22, 2014 at 07:16:47PM -0700, Tom Herbert wrote:
[...]

Alexei, I believe you said previously said that SW should not dictate
HW models. I agree with this, but also believe the converse is true--
HW shouldn't dictate SW model. This is really why I'm raising the
question of what it means to integrate a switch into the host stack.

Tom, when I read this I cannot help but remind myself that the
intentions/hopes/dreams of those on this thread and how different their
views can be on what it means to add additional 'offload support' to the
kernel.

There are clearly some that are most interested in how an eSwitch on an
SR-IOV capable NIC be controlled can provide traditional forwarding help
as well as offload the various technologies they hope to terminate
at/inside their endpoint (host/guest/container) -- Thomas's _simple_
use-case demonstrates this. ;)  This is a logical extention/increase in
functionality that is offered in many eSwitches that was previously
hidden from the user with the first generation SR-IOV capable network
devices on hosts/servers.


Indeed.

The idea is to leverage OVS to manage eSwitch (embedded NIC switch) as 
well (NOT to offload OVS).


We envision a seamless integration of user environment which is based on 
OVS with SRIOV eSwitch and the grounds for that were very well supported 
in Jiri’s V1.


The eSwitch hardware does not need to have multiple tables and ‘enjoys’ 
the flat rule of OVS. The kernel datapath does not need to be aware of 
the existence of HW nor its capabilities, it just pushes the flow also 
to the switchdev which represents the eSwitch.


If the flow can be supported in HW it will be forwarded in HW and if not 
it will be forwarded by the kernel



[]

And now we also have the patchset that spawned what I think has been
more excellent discussion.  Jiri and Scott's patches bring up another,
more generic model that while not currently backed by hardware provided
an example/vision for what could be done if such hardware existed and
how to consider interacting with that driver/hardware (that clearly has
been met with some resistance, but the discussion has been great).
There ultimate goals appear to be similar to those that want full
offload/fordwarding support for a device, but via a different method
than what some would consider standard.

I am personally hopeful that most who are passionate about this will be
able to get together next month at LPC (or send someone to represent
them!) so that those interested can sit in the same room and try to
better understand each others desires and start to form some concrete
direction towards a solution that seems to meet the needs of most while
not being an architectural disaster.



Yep. LPC is the time and place to go over the multiple use-cases 
(phyiscal switch, eSwitch, eBPF, etc) that could (should) be supported 
by the basic framework.


Or.

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [patch net-next RFC 02/12] net: rename netdev_phys_port_id to more generic name

2014-08-26 Thread Or Gerlitz 

On 21/08/2014 19:18, Jiri Pirko wrote:

--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -868,7 +868,7 @@ static noinline size_t if_nlmsg_size(const struct 
net_device *dev,
   + rtnl_port_size(dev, ext_filter_mask) /* IFLA_VF_PORTS + 
IFLA_PORT_SELF */
   + rtnl_link_get_size(dev) /* IFLA_LINKINFO */
   + rtnl_link_get_af_size(dev) /* IFLA_AF_SPEC */
-  + nla_total_size(MAX_PHYS_PORT_ID_LEN); /* IFLA_PHYS_PORT_ID */
+  + nla_total_size(MAX_PHYS_ITEM_ID_LEN); /* IFLA_PHYS_PORT_ID */
  }

  static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev)
@@ -952,7 +952,7 @@ static int rtnl_port_fill(struct sk_buff *skb, struct 
net_device *dev,
  static int rtnl_phys_port_id_fill(struct sk_buff *skb, struct net_device *dev)
  {
int err;
-   struct netdev_phys_port_id ppid;
+   struct netdev_phys_item_id ppid;

err = dev_get_phys_port_id(dev, ppid);
if (err) {
@@ -1196,7 +1196,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
[IFLA_PROMISCUITY]  = { .type = NLA_U32 },
[IFLA_NUM_TX_QUEUES]= { .type = NLA_U32 },
[IFLA_NUM_RX_QUEUES]= { .type = NLA_U32 },
-   [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = 
MAX_PHYS_PORT_ID_LEN },
+   [IFLA_PHYS_PORT_ID] = { .type = NLA_BINARY, .len = 
MAX_PHYS_ITEM_ID_LEN },
[IFLA_CARRIER_CHANGES]  = { .type = NLA_U32 },  /* ignored */
  };



just a nit, but if this approach/patch goes in, any reason not to change 
IFLA_PHYS_PORT_ID to IFLA_PHYS_ITEM_ID?


Or.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [GIT net] Open vSwitch

2014-02-05 Thread Or Gerlitz 
On Wed, Feb 5, 2014 at 8:59 AM, Jesse Gross je...@nicira.com wrote:

 A handful of bug fixes for net/3.14. High level fixes are:
  * Regressions introduced by the zerocopy changes, particularly with
old userspaces.

Hi, so this post was the 2nd version of the five patches you posted
earlier, right? it would be very helpful if you
denote that on the subject line (e.g just use --subject-prefix=PATCH
net V1 for git format-patch) and provide
crash diff listing from V0. Also, on a related note to the patch that
deals with locking, I see these two smatch
complaints, which might be false-positives, what's your thinking?

net/openvswitch/flow.c:127 ovs_flow_stats_get() warn: returning with
unbalanced local_bh_disable
net/openvswitch/flow.c:160 ovs_flow_stats_clear() warn: returning with
unbalanced local_bh_disable
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH net-next v2 00/12] openvswitch: gre tunneling support.

2013-06-18 Thread Or Gerlitz 
On Tue, Jun 18, 2013 at 3:49 AM, Pravin B Shelar pshe...@nicira.com wrote:
 Following patch series adds support for gre tunneling.
 First six patches extend kernel gre and ip_tunnel modules
 api so that there is more code sharing between gre modules
 and ovs. Rest of patches adds ovs tunneling infrastructre
 and gre protocol vport.

 V2 fixes two patches according to comments from Jesse.

Hi Pravin,

I don't know what is the exact volume of changes between v1/v2 in this
case, but generally,
it would be very helpful if you make sure to spare few words on the
actual changes between versions.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 0/5] ipv4_tunnels: Modularize ipv4 tunneling.

2013-01-08 Thread Or Gerlitz 
On Tue, Jan 8, 2013 at 4:31 AM, Pravin B Shelar pshe...@nicira.com wrote:
 Following patch series restructure GRE and IPIP tunneling code
 to make it modular. It adds ip_tunnel module which acts as
 generic tunneling layer which has common code. I have patch
 to do same for VXLAN too. [..]

Can you be more specific re the vxlan patch?

Or.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev