Skip to content

Commit

Permalink
netdev-offload-tc: Revert tunnel src/dst port masks handling
Browse files Browse the repository at this point in the history 
The cited commit intended to add tc support for masking tunnel src/dst
ips and ports. It's not possible to do tunnel ports masking with
openflow rules and the default mask for tunnel ports set to 0 in
tnl_wc_init(), unlike tunnel ports default mask which is full mask.
So instead of never passing tunnel ports to tc, revert the changes
to tunnel ports to always pass the tunnel port.
In sw classification is done by the kernel, but for hw we must match
the tunnel dst port.

Fixes: 5f568d0 ("netdev-offload-tc: Allow to match the IP and port mask of tunnel")
Signed-off-by: Roi Dayan <[email protected]>
Reviewed-by: Eli Britstein <[email protected]>
Signed-off-by: Simon Horman <[email protected]>
  • Loading branch information
@roidayan @shorman-netronome
roidayan authored and shorman-netronome committed Jun 19, 2020
1 parent fecb280 commit 1fe4297
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 57 deletions.
2 changes: 0 additions & 2 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ Post-v2.13.0
- Tunnels: TC Flower offload
* Tunnel Local endpoint address masked match are supported.
* Tunnel Romte endpoint address masked match are supported.
* Tunnel Local endpoint ports masked match are supported.
* Tunnel Romte endpoint ports masked match are supported.


v2.13.0 - 14 Feb 2020
Expand Down
3 changes: 0 additions & 3 deletions include/openvswitch/match.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ void match_set_tun_flags(struct match *match, uint16_t flags);
void match_set_tun_flags_masked(struct match *match, uint16_t flags, uint16_t mask);
void match_set_tun_tp_dst(struct match *match, ovs_be16 tp_dst);
void match_set_tun_tp_dst_masked(struct match *match, ovs_be16 port, ovs_be16 mask);
void match_set_tun_tp_src(struct match *match, ovs_be16 tp_src);
void match_set_tun_tp_src_masked(struct match *match,
ovs_be16 port, ovs_be16 mask);
void match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16 mask);
void match_set_tun_gbp_id(struct match *match, ovs_be16 gbp_id);
void match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, uint8_t mask);
Expand Down
13 changes: 0 additions & 13 deletions lib/match.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,19 +293,6 @@ match_set_tun_tp_dst_masked(struct match *match, ovs_be16 port, ovs_be16 mask)
match->flow.tunnel.tp_dst = port & mask;
}

void
match_set_tun_tp_src(struct match *match, ovs_be16 tp_src)
{
match_set_tun_tp_src_masked(match, tp_src, OVS_BE16_MAX);
}

void
match_set_tun_tp_src_masked(struct match *match, ovs_be16 port, ovs_be16 mask)
{
match->wc.masks.tunnel.tp_src = mask;
match->flow.tunnel.tp_src = port & mask;
}

void
match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16 mask)
{
Expand Down
13 changes: 2 additions & 11 deletions lib/netdev-offload-tc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -712,15 +712,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
match_set_tun_ttl_masked(match, flower->key.tunnel.ttl,
flower->mask.tunnel.ttl);
}
if (flower->mask.tunnel.tp_dst) {
match_set_tun_tp_dst_masked(match,
flower->key.tunnel.tp_dst,
flower->mask.tunnel.tp_dst);
}
if (flower->mask.tunnel.tp_src) {
match_set_tun_tp_src_masked(match,
flower->key.tunnel.tp_src,
flower->mask.tunnel.tp_src);
if (flower->key.tunnel.tp_dst) {
match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst);
}
if (flower->key.tunnel.metadata.present.len) {
flower_tun_opt_to_match(match, flower);
Expand Down Expand Up @@ -1470,8 +1463,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
flower.mask.tunnel.ipv6.ipv6_dst = tnl_mask->ipv6_dst;
flower.mask.tunnel.tos = tnl_mask->ip_tos;
flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
flower.mask.tunnel.tp_src = tnl_mask->tp_src;
flower.mask.tunnel.tp_dst = tnl_mask->tp_dst;
flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0;
flower_match_to_tun_opt(&flower, tnl, tnl_mask);
flower.tunnel = true;
Expand Down
28 changes: 2 additions & 26 deletions lib/tc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,6 @@ static const struct nl_policy tca_flower_policy[] = {
.optional = true, },
[TCA_FLOWER_KEY_ENC_UDP_DST_PORT] = { .type = NL_A_U16,
.optional = true, },
[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT] = { .type = NL_A_U16,
.optional = true, },
[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK] = { .type = NL_A_U16,
.optional = true, },
[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK] = { .type = NL_A_U16,
.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_IP_TTL] = { .type = NL_A_U8,
Expand Down Expand Up @@ -746,15 +740,7 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower)
flower->key.tunnel.ipv6.ipv6_dst =
nl_attr_get_in6_addr(attrs[TCA_FLOWER_KEY_ENC_IPV6_DST]);
}
if (attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]) {
flower->mask.tunnel.tp_src =
nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]);
flower->key.tunnel.tp_src =
nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT]);
}
if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]) {
flower->mask.tunnel.tp_dst =
nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]);
if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]) {
flower->key.tunnel.tp_dst =
nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
}
Expand Down Expand Up @@ -2713,10 +2699,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
struct in6_addr *ipv6_dst_mask = &flower->mask.tunnel.ipv6.ipv6_dst;
struct in6_addr *ipv6_src = &flower->key.tunnel.ipv6.ipv6_src;
struct in6_addr *ipv6_dst = &flower->key.tunnel.ipv6.ipv6_dst;
ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst;
ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src;
ovs_be16 tp_dst = flower->key.tunnel.tp_dst;
ovs_be16 tp_src = flower->key.tunnel.tp_src;
ovs_be32 id = be64_to_be32(flower->key.tunnel.id);
uint8_t tos = flower->key.tunnel.tos;
uint8_t ttl = flower->key.tunnel.ttl;
Expand Down Expand Up @@ -2748,16 +2731,9 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
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);
}
if (tp_dst_mask) {
nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
tp_dst_mask);
if (tp_dst) {
nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
}
if (tp_src_mask) {
nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK,
tp_src_mask);
nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT, tp_src);
}
if (id_mask) {
nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
}
Expand Down
4 changes: 2 additions & 2 deletions tests/tunnel.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ AT_CHECK([ovs-appctl dpif/show | tail -n +3], [0], [dnl
p2 2/2: (dummy)
])

AT_CHECK([ovs-appctl dpctl/add-flow "tunnel(dst=1.1.1.1,src=3.3.3.200/255.255.255.0,tp_dst=123,tp_src=1/0xf,ttl=64),recirc_id(0),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"])
AT_CHECK([ovs-appctl dpctl/add-flow "tunnel(dst=1.1.1.1,src=3.3.3.200/255.255.255.0,tp_dst=123,tp_src=1,ttl=64),recirc_id(0),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"])

AT_CHECK([ovs-appctl dpctl/dump-flows | tail -1], [0], [dnl
tunnel(src=3.3.3.200/255.255.255.0,dst=1.1.1.1,ttl=64,tp_src=1/0xf,tp_dst=123),recirc_id(0),in_port(1),eth_type(0x0800), packets:0, bytes:0, used:never, actions:2
tunnel(src=3.3.3.200/255.255.255.0,dst=1.1.1.1,ttl=64,tp_src=1,tp_dst=123),recirc_id(0),in_port(1),eth_type(0x0800), packets:0, bytes:0, used:never, actions:2
])

OVS_VSWITCHD_STOP
Expand Down

0 comments on commit 1fe4297

Please sign in to comment.