diff mbox series

[v2] net/tap: fix zeroed flow mask configurations

Message ID 1533553127-18664-1-git-send-email-matan@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] net/tap: fix zeroed flow mask configurations |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Matan Azrad Aug. 6, 2018, 10:58 a.m. UTC 
  The rte_flow meaning of zero flow mask configuration is to match all
the range of the item value.
For example, the flow eth / ipv4 dst spec 1.2.3.4 dst mask 0.0.0.0
should much all the ipv4 traffic from the rte_flow API perspective.

From some kernel perspectives the above rule means to ignore all the
ipv4 traffic (e.g. Ubuntu 16.04, 4.15.10).

Due to the fact that the tap PMD should provide the rte_flow meaning,
it is necessary to ignore the spec in case the mask is zero when it
forwards such like flows to the kernel.
So, the above rule should be translated to eth / ipv4 to get the
correct meaning.

Ignore spec configurations when the mask is zero.

Fixes: de96fe68ae95 ("net/tap: add basic flow API patterns and actions")
Cc: stable@dpdk.org

Signed-off-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/tap/tap_flow.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

V2:
Address Adrien comments to fix also the spec=0 check.

Comments

Adrien Mazarguil Aug. 6, 2018, 1:16 p.m. UTC | #1 
On Mon, Aug 06, 2018 at 10:58:47AM +0000, Matan Azrad wrote:
> The rte_flow meaning of zero flow mask configuration is to match all
> the range of the item value.
> For example, the flow eth / ipv4 dst spec 1.2.3.4 dst mask 0.0.0.0
> should much all the ipv4 traffic from the rte_flow API perspective.
> 
> From some kernel perspectives the above rule means to ignore all the
> ipv4 traffic (e.g. Ubuntu 16.04, 4.15.10).
> 
> Due to the fact that the tap PMD should provide the rte_flow meaning,
> it is necessary to ignore the spec in case the mask is zero when it
> forwards such like flows to the kernel.
> So, the above rule should be translated to eth / ipv4 to get the
> correct meaning.
> 
> Ignore spec configurations when the mask is zero.
> 
> Fixes: de96fe68ae95 ("net/tap: add basic flow API patterns and actions")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> ---
>  drivers/net/tap/tap_flow.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> V2:
> Address Adrien comments to fix also the spec=0 check.

Thanks,

Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Thomas Monjalon Aug. 7, 2018, 2:01 p.m. UTC | #2 
06/08/2018 15:16, Adrien Mazarguil:
> On Mon, Aug 06, 2018 at 10:58:47AM +0000, Matan Azrad wrote:
> > The rte_flow meaning of zero flow mask configuration is to match all
> > the range of the item value.
> > For example, the flow eth / ipv4 dst spec 1.2.3.4 dst mask 0.0.0.0
> > should much all the ipv4 traffic from the rte_flow API perspective.
> > 
> > From some kernel perspectives the above rule means to ignore all the
> > ipv4 traffic (e.g. Ubuntu 16.04, 4.15.10).
> > 
> > Due to the fact that the tap PMD should provide the rte_flow meaning,
> > it is necessary to ignore the spec in case the mask is zero when it
> > forwards such like flows to the kernel.
> > So, the above rule should be translated to eth / ipv4 to get the
> > correct meaning.
> > 
> > Ignore spec configurations when the mask is zero.
> > 
> > Fixes: de96fe68ae95 ("net/tap: add basic flow API patterns and actions")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Matan Azrad <matan@mellanox.com>
> > ---
> >  drivers/net/tap/tap_flow.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > V2:
> > Address Adrien comments to fix also the spec=0 check.
> 
> Thanks,
> 
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Applied, thanks
diff mbox series

Patch

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 6b60e6d..0e01af6 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -537,7 +537,7 @@  tap_flow_create_eth(const struct rte_flow_item *item, void *data)
 	if (!flow)
 		return 0;
 	msg = &flow->msg;
-	if (!is_zero_ether_addr(&spec->dst)) {
+	if (!is_zero_ether_addr(&mask->dst)) {
 		tap_nlattr_add(&msg->nh, TCA_FLOWER_KEY_ETH_DST, ETHER_ADDR_LEN,
 			   &spec->dst.addr_bytes);
 		tap_nlattr_add(&msg->nh,
@@ -651,13 +651,13 @@  tap_flow_create_ipv4(const struct rte_flow_item *item, void *data)
 		info->eth_type = htons(ETH_P_IP);
 	if (!spec)
 		return 0;
-	if (spec->hdr.dst_addr) {
+	if (mask->hdr.dst_addr) {
 		tap_nlattr_add32(&msg->nh, TCA_FLOWER_KEY_IPV4_DST,
 			     spec->hdr.dst_addr);
 		tap_nlattr_add32(&msg->nh, TCA_FLOWER_KEY_IPV4_DST_MASK,
 			     mask->hdr.dst_addr);
 	}
-	if (spec->hdr.src_addr) {
+	if (mask->hdr.src_addr) {
 		tap_nlattr_add32(&msg->nh, TCA_FLOWER_KEY_IPV4_SRC,
 			     spec->hdr.src_addr);
 		tap_nlattr_add32(&msg->nh, TCA_FLOWER_KEY_IPV4_SRC_MASK,
@@ -707,13 +707,13 @@  tap_flow_create_ipv6(const struct rte_flow_item *item, void *data)
 		info->eth_type = htons(ETH_P_IPV6);
 	if (!spec)
 		return 0;
-	if (memcmp(spec->hdr.dst_addr, empty_addr, 16)) {
+	if (memcmp(mask->hdr.dst_addr, empty_addr, 16)) {
 		tap_nlattr_add(&msg->nh, TCA_FLOWER_KEY_IPV6_DST,
 			   sizeof(spec->hdr.dst_addr), &spec->hdr.dst_addr);
 		tap_nlattr_add(&msg->nh, TCA_FLOWER_KEY_IPV6_DST_MASK,
 			   sizeof(mask->hdr.dst_addr), &mask->hdr.dst_addr);
 	}
-	if (memcmp(spec->hdr.src_addr, empty_addr, 16)) {
+	if (memcmp(mask->hdr.src_addr, empty_addr, 16)) {
 		tap_nlattr_add(&msg->nh, TCA_FLOWER_KEY_IPV6_SRC,
 			   sizeof(spec->hdr.src_addr), &spec->hdr.src_addr);
 		tap_nlattr_add(&msg->nh, TCA_FLOWER_KEY_IPV6_SRC_MASK,
@@ -762,10 +762,10 @@  tap_flow_create_udp(const struct rte_flow_item *item, void *data)
 	tap_nlattr_add8(&msg->nh, TCA_FLOWER_KEY_IP_PROTO, IPPROTO_UDP);
 	if (!spec)
 		return 0;
-	if (spec->hdr.dst_port & mask->hdr.dst_port)
+	if (mask->hdr.dst_port)
 		tap_nlattr_add16(&msg->nh, TCA_FLOWER_KEY_UDP_DST,
 			     spec->hdr.dst_port);
-	if (spec->hdr.src_port & mask->hdr.src_port)
+	if (mask->hdr.src_port)
 		tap_nlattr_add16(&msg->nh, TCA_FLOWER_KEY_UDP_SRC,
 			     spec->hdr.src_port);
 	return 0;
@@ -808,10 +808,10 @@  tap_flow_create_tcp(const struct rte_flow_item *item, void *data)
 	tap_nlattr_add8(&msg->nh, TCA_FLOWER_KEY_IP_PROTO, IPPROTO_TCP);
 	if (!spec)
 		return 0;
-	if (spec->hdr.dst_port & mask->hdr.dst_port)
+	if (mask->hdr.dst_port)
 		tap_nlattr_add16(&msg->nh, TCA_FLOWER_KEY_TCP_DST,
 			     spec->hdr.dst_port);
-	if (spec->hdr.src_port & mask->hdr.src_port)
+	if (mask->hdr.src_port)
 		tap_nlattr_add16(&msg->nh, TCA_FLOWER_KEY_TCP_SRC,
 			     spec->hdr.src_port);
 	return 0;