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.ri...@6wind.com>
> > Acked-by: Olga Shern <ol...@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
