From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gaetan.rivet@6wind.com>
Received: from mail-wr0-f179.google.com (mail-wr0-f179.google.com
 [209.85.128.179]) by dpdk.org (Postfix) with ESMTP id F29F5376C
 for <dev@dpdk.org>; Tue, 18 Jul 2017 00:48:09 +0200 (CEST)
Received: by mail-wr0-f179.google.com with SMTP id y43so3637204wrd.3
 for <dev@dpdk.org>; Mon, 17 Jul 2017 15:48:09 -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:content-transfer-encoding:in-reply-to
 :user-agent; bh=2P8M6b8UCED6SRnBQqVjb0v6gnCLQBmEBvGiKqklojM=;
 b=claxq0Qt1jIivEV0tVJiW/7JixsLSX+E1aKX0qyK9ZJRt5PPbnyvKLfELArH7tgsHx
 ojrZJHQgDC9eRxMXM0r9mH+xbfvKBzfeiwRAoJnz/X0MvYFuMju71EAY4tDS9dU7Y6uy
 SJhRDYgSPIp3669gGvgw3AXgyqpRz6sn6C2lztueGnkFm6ltaHGdxXLOh0zn3ygRSuEh
 FzTO5WVExefujgqVNv6OzMBzxD3n4Y/L4hQgeub4kkMw9e5VDGGIOQRsXM29JYr23uWJ
 NGxh5N0Rvi7LaKzBprFdn/ztlKc9M4NN5XDHLmNe1imKfdDki6USN+fO+IdfcpghSZmh
 4RuA==
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:content-transfer-encoding
 :in-reply-to:user-agent;
 bh=2P8M6b8UCED6SRnBQqVjb0v6gnCLQBmEBvGiKqklojM=;
 b=lKSE1JdfwaPvMpPwWpvORRWvnR3UHz+31F9tSCILQDUeVqKCeJ6ZGSS+8SzWhh8pNX
 j5kSnvJPQuuhbm/YZrXs3pzjKgEfulYdD3t6YN/jOBTCt+BRIKW0AHLZd7K27fmZVifa
 m4vI1PSx1OALKx4aWOOqDKnrM0n/cfYl7QrspwIiwVvV4y4qW2UuSM1eFGZAjv+WqIjK
 iPWziTTeCgV/76fqYl6M8i2n0E7KySaWJGP5z+8BP5Ot5UFm2AUM8wFr+5YMVLhhnEzF
 OtCrWkR3C25LBw5EOWdWELs7fDHHgbvkxV4XsBzYg/k1iBrBFWdr7N4fvc9fo7x++7ZG
 bxHw==
X-Gm-Message-State: AIVw111VCbmjt3QveWcXNPGQ17/ySGYdcRn9Mlp1084EZTwCCDGjFZa1
 5ylzqkiAW3PTh0tCmyU=
X-Received: by 10.28.4.78 with SMTP id 75mr15729wme.66.1500331689439;
 Mon, 17 Jul 2017 15:48:09 -0700 (PDT)
Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com.
 [62.23.145.78])
 by smtp.gmail.com with ESMTPSA id p140sm15248954wmb.28.2017.07.17.15.48.07
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Mon, 17 Jul 2017 15:48:08 -0700 (PDT)
Date: Tue, 18 Jul 2017 00:47:59 +0200
From: =?iso-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org
Message-ID: <20170717224759.GU11154@bidouze.vm.6wind.com>
References: <cover.1500130634.git.gaetan.rivet@6wind.com>
 <b822c60f9c1f2d1e4a7538b9b5a77a7a259aa514.1500130635.git.gaetan.rivet@6wind.com>
 <5d9f75d9-a7a6-1035-fb05-a87f1b071e0a@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <5d9f75d9-a7a6-1035-fb05-a87f1b071e0a@intel.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Subject: Re: [dpdk-dev] [PATCH v10 07/11] net/failsafe: support offload
 capabilities
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 17 Jul 2017 22:48:10 -0000

On Mon, Jul 17, 2017 at 05:22:15PM +0100, Ferruh Yigit wrote:
> On 7/15/2017 6:57 PM, Gaetan Rivet wrote:
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > Acked-by: Olga Shern <olgas@mellanox.com>
> > ---
> >  doc/guides/nics/features/failsafe.ini |   6 ++
> >  drivers/net/failsafe/failsafe_ops.c   | 131 +++++++++++++++++++++++++++++++++-
> >  2 files changed, 135 insertions(+), 2 deletions(-)
> > 
> > diff --git a/doc/guides/nics/features/failsafe.ini b/doc/guides/nics/features/failsafe.ini
> > index 9167b59..257f579 100644
> > --- a/doc/guides/nics/features/failsafe.ini
> > +++ b/doc/guides/nics/features/failsafe.ini
> > @@ -14,6 +14,12 @@ Unicast MAC filter   = Y
> >  Multicast MAC filter = Y
> >  VLAN filter          = Y
> >  Flow API             = Y
> > +VLAN offload         = Y
> > +QinQ offload         = Y
> > +L3 checksum offload  = Y
> > +L4 checksum offload  = Y
> > +Inner L3 checksum    = Y
> > +Inner L4 checksum    = Y
> 
> As previous comment on features, these are advertised as supported but
> depends on sub-devices.
> 
> Overall I don't know what does these mean for failsafe like abstract device.
> 

We should look into this.

The rationale for features in fail-safe was that

- If the slave supported the feature, and using it with the fail-safe
  did not impair the feature, then the fail-safe was transparent as far
  as this feature was concerned --> support = Y.

  Meaning that users would be able to use this feature with the
  fail-safe.

- If any slave did not support the feature, it would be dynamically
  disabled when it made sense, or ENOTSUPP is returned if an ops is
  missing (checked automatically by calling the ether API).

In the end, the feature matrix is used by the user to check whether the
feature is available using this PMD. Disabling all features in the
matrix for the fail-safe does not help the user at all, as they should
then check manually / look at the code.

On the other hand, the possible issue is that some users might expect
that the fail-safe could emulate in software missing features. The
usefullness of having the matrix to compare against other PMDs features
outweight this possibility as I find this assumption far-fetched, but I
can always remove the feature support for the moment and we will see
afterward how to deal with it.

> >  Packet type parsing  = Y
> >  Basic stats          = Y
> >  Stats per queue      = Y
> > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c
> > index 0c8aa35..654b411 100644
> > --- a/drivers/net/failsafe/failsafe_ops.c
> > +++ b/drivers/net/failsafe/failsafe_ops.c
> > @@ -64,22 +64,149 @@ static struct rte_eth_dev_info default_infos = {
> >  		.nb_seg_max = UINT16_MAX,
> >  		.nb_mtu_seg_max = UINT16_MAX,
> >  	},
> > -	/* Set of understood capabilities */
> > -	.rx_offload_capa = 0x0,
> > +	/*
> > +	 * Set of capabilities that can be verified upon
> > +	 * configuring a sub-device.
> > +	 */
> > +	.rx_offload_capa =
> > +		DEV_RX_OFFLOAD_VLAN_STRIP |
> > +		DEV_RX_OFFLOAD_QINQ_STRIP |
> > +		DEV_RX_OFFLOAD_IPV4_CKSUM |
> > +		DEV_RX_OFFLOAD_UDP_CKSUM |
> > +		DEV_RX_OFFLOAD_TCP_CKSUM |
> > +		DEV_RX_OFFLOAD_TCP_LRO,
> 
> These are not dynamic, even though some may be disabled via
> fs_port_disable_offload() same these values will be returned to the
> application, which is wrong.
> 

These are the default rx_offload_capa. This flag value is AND-ed with
that of the slaves before being returned. A slave is assured to be
present at all time, so it will be restricted to the common set between
the fail-safe and the slave features.

> >  	.tx_offload_capa = 0x0,
> 
> Claiming support for most of the offloads means supporting it both for
> Rx and Tx path. This patch only takes account the Rx ones.
> 

Ah I did not know. I will remove the feature support for offloads, the same
work that was done with Rx will be done with Tx.

> >  	.flow_type_rss_offloads = 0x0,
> >  };
> > 
> 
> <...>
> 

-- 
Gaƫtan Rivet
6WIND