From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com
 [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id D6E331B3A7
 for <stable@dpdk.org>; Thu,  2 Aug 2018 16:27:54 +0200 (CEST)
Received: by mail-wr1-f65.google.com with SMTP id b15-v6so2328217wrv.10
 for <stable@dpdk.org>; Thu, 02 Aug 2018 07:27:54 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=6wind-com.20150623.gappssmtp.com; s=20150623;
 h=date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:in-reply-to;
 bh=2qEZxO75ZjZZyPCM63Zc+wIc60ipiQOgf+flq8YgQsc=;
 b=AzkdWNnZ/RgX61p0GFYMRPHn62zeN3iYzPPS6NQzlrSytNS2ITxZxRVzC24JSthByO
 AEcqhkcNrKdp6XbkbJqwgAYz94tILMj9pR0gap+VdS3Lgz/49mMxmoTujQp8wAj1NODZ
 tfe4RapylwTXkAqhASEa7z6P2LUBOUVIpsU03wyfFu341m7JwEKK5Fro+JfbM+cpZHoD
 4PORDHpFnOnYTYX+pomcABB0L65831R+MIDcxZYzlR0tuIqEC1MFE4d/FcBw5NjOkz7V
 1VN+3mzvwGVPjNWUXAvtC/rpoW8WbmE9zZKbKKKzqton6vD7i7MY1oV9EhbxPHNPN4Ab
 pP1w==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to;
 bh=2qEZxO75ZjZZyPCM63Zc+wIc60ipiQOgf+flq8YgQsc=;
 b=jSYQwwrR/Gin+zKjweC0edjwPKp5mRjQyQDep1Q8nAtQPoeI5PKTzvo0T6o0+UZvio
 vcmGWVI2/nZoufs2v34uCRItD/aiRURif/ve3KgnvgS3cpnz/QB61Qz6QeL+HcYnRrCB
 Wky+mOzWXm7sWg2kaZZ8asw+k4tNJ7/XSiKjN3lo6QoLlVOJUgNsUmICbQCx+7A4SIYm
 ztw76wSKEl3XVbVzcV0xfxO3RbpDqlGLcVsomYa0u/bjIS4dmdgJEqme2txYSup8JGXh
 pzevACR2sWGv/yr3iP54Bhlci/ogIY8NgUg2eCiaW5r7LFrLEetIGL/9do0qW/PxMwz5
 PLfQ==
X-Gm-Message-State: AOUpUlH/pzwb4yVE4QYMZOfDCXuHMNM77QDZIenOGKddEgdg22rLPAOB
 5GbZUa0QsCPsVqliHp2NlpMn5w==
X-Google-Smtp-Source: AAOMgpey0v7q3+ZMNZOW53Wn9KtitytCs91WPvYj086+YYz6rtZngYePYDtO9TLQue25YuG1Ze1LQg==
X-Received: by 2002:adf:9954:: with SMTP id
 x78-v6mr2318741wrb.178.1533220074623; 
 Thu, 02 Aug 2018 07:27:54 -0700 (PDT)
Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78])
 by smtp.gmail.com with ESMTPSA id u7-v6sm2787852wmd.46.2018.08.02.07.27.53
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Thu, 02 Aug 2018 07:27:53 -0700 (PDT)
Date: Thu, 2 Aug 2018 16:27:37 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Matan Azrad <matan@mellanox.com>
Cc: Keith Wiles <keith.wiles@intel.com>, Ophir Munk <ophirmu@mellanox.com>,
 dev@dpdk.org, stable@dpdk.org
Message-ID: <20180802142737.GO5211@6wind.com>
References: <1533205980-7874-1-git-send-email-matan@mellanox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1533205980-7874-1-git-send-email-matan@mellanox.com>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/tap: fix zeroed flow mask
	configurations
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 02 Aug 2018 14:27:55 -0000

On Thu, Aug 02, 2018 at 10:33:00AM +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.

I would go further, one should be able to match IP address 0.0.0.0 for
instance. The PMD should only trust the mask on all fields without looking
at spec. See below for suggestions.

> 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 | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
> index 6b60e6d..993e6f6 100644
> --- a/drivers/net/tap/tap_flow.c
> +++ b/drivers/net/tap/tap_flow.c
> @@ -537,7 +537,8 @@ 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(&spec->dst) &&

This check should be removed.

> +	    !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 +652,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 (spec->hdr.dst_addr && mask->hdr.dst_addr) {

Ditto (before &&).

>  		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 (spec->hdr.src_addr && mask->hdr.src_addr) {

Ditto.

>  		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 +708,15 @@ 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(spec->hdr.dst_addr, empty_addr, 16) &&

Ditto.

> +	    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(spec->hdr.src_addr, empty_addr, 16) &&

Ditto.

> +	    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,
> -- 
> 2.7.4
> 

-- 
Adrien Mazarguil
6WIND