Skip to content

Commit

Permalink
dpif-netdev: Don't use zero flow mark.
Browse files Browse the repository at this point in the history
Zero flow mark is used to indicate the HW to remove the mark. A packet
marked with zero mark is received in SW without a mark at all, so it
cannot be used as a valid mark. Change the pool range to fix it.

Fixes: 241bad1 ("dpif-netdev: associate flow with a mark id")
Signed-off-by: Eli Britstein <elibr@mellanox.com>
Reviewed-by: Roni Bar Yanai <roniba@mellanox.com>
Acked-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
Eli Britstein authored and igsilya committed Jul 9, 2020
1 parent 39cc59d commit 9747d56
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
8 changes: 6 additions & 2 deletions lib/dpif-netdev.c
Expand Up @@ -2149,7 +2149,11 @@ dp_netdev_pmd_find_dpcls(struct dp_netdev_pmd_thread *pmd,
}

#define MAX_FLOW_MARK (UINT32_MAX - 1)
#define INVALID_FLOW_MARK (UINT32_MAX)
#define INVALID_FLOW_MARK 0
/* Zero flow mark is used to indicate the HW to remove the mark. A packet
* marked with zero mark is received in SW without a mark at all, so it
* cannot be used as a valid mark.
*/

struct megaflow_to_mark_data {
const struct cmap_node node;
Expand All @@ -2175,7 +2179,7 @@ flow_mark_alloc(void)

if (!flow_mark.pool) {
/* Haven't initiated yet, do it here */
flow_mark.pool = id_pool_create(0, MAX_FLOW_MARK);
flow_mark.pool = id_pool_create(1, MAX_FLOW_MARK);
}

if (id_pool_alloc_id(flow_mark.pool, &mark)) {
Expand Down
12 changes: 6 additions & 6 deletions tests/dpif-netdev.at
Expand Up @@ -393,7 +393,7 @@ skb_priority(0),skb_mark(0),ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),recirc
# Check that flow successfully offloaded.
OVS_WAIT_UNTIL([grep "succeed to add netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: flow put[[create]]: flow match: recirc_id=0,eth,ip,in_port=1,vlan_tci=0x0000,nw_frag=no, mark: 0
p1: flow put[[create]]: flow match: recirc_id=0,eth,ip,in_port=1,vlan_tci=0x0000,nw_frag=no, mark: 1
])
# Check that datapath flow installed successfully.
AT_CHECK([filter_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
Expand All @@ -404,7 +404,7 @@ recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), a

# Check for succesfull packet matching with installed offloaded flow.
AT_CHECK([filter_hw_packet_netdev_dummy < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: packet: ip,vlan_tci=0x0000,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=64 matches with flow: recirc_id=0,eth,ip,vlan_tci=0x0000,nw_frag=no with mark: 0
p1: packet: ip,vlan_tci=0x0000,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_proto=0,nw_tos=0,nw_ecn=0,nw_ttl=64 matches with flow: recirc_id=0,eth,ip,vlan_tci=0x0000,nw_frag=no with mark: 1
])

ovs-appctl revalidator/wait
Expand All @@ -421,7 +421,7 @@ recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), p
# Check that flow successfully deleted from HW.
OVS_WAIT_UNTIL([grep "succeed to delete netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_del < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: flow del: mark: 0
p1: flow del: mark: 1
])
OVS_VSWITCHD_STOP
AT_CLEANUP])
Expand Down Expand Up @@ -460,7 +460,7 @@ packet_type(ns=0,id=0),eth(src=00:06:07:08:09:0a,dst=00:01:02:03:04:05),eth_type
# Check that flow successfully offloaded.
OVS_WAIT_UNTIL([grep "succeed to add netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: flow put[[create]]: flow match: recirc_id=0,eth,udp,in_port=1,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82, mark: 0
p1: flow put[[create]]: flow match: recirc_id=0,eth,udp,in_port=1,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82, mark: 1
])
# Check that datapath flow installed successfully.
AT_CHECK([filter_flow_install < ovs-vswitchd.log | strip_xout], [0], [dnl
Expand All @@ -472,7 +472,7 @@ recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x8100),vlan(vid=99,pcp=
# Check for succesfull packet matching with installed offloaded flow.
AT_CHECK([filter_hw_packet_netdev_dummy < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: packet: udp,dl_vlan=99,dl_vlan_pcp=7,vlan_tci1=0x0000,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=127.0.0.1,nw_dst=127.0.0.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=81,tp_dst=82 dnl
matches with flow: recirc_id=0,eth,udp,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82 with mark: 0
matches with flow: recirc_id=0,eth,udp,dl_vlan=99,dl_vlan_pcp=7,nw_src=127.0.0.1,nw_frag=no,tp_dst=82 with mark: 1
])

ovs-appctl revalidator/wait
Expand All @@ -490,7 +490,7 @@ packets:1, bytes:64, used:0.0s, actions:set(ipv4(src=192.168.0.7)),set(udp(dst=3
# Check that flow successfully deleted from HW.
OVS_WAIT_UNTIL([grep "succeed to delete netdev flow" ovs-vswitchd.log])
AT_CHECK([filter_hw_flow_del < ovs-vswitchd.log | strip_xout], [0], [dnl
p1: flow del: mark: 0
p1: flow del: mark: 1
])

# Check that ip address and udp port were correctly modified in output packets.
Expand Down

0 comments on commit 9747d56

Please sign in to comment.