[dpdk-dev,v10,07/11] net/failsafe: support offload capabilities
Checks
Commit Message
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(-)
Comments
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.
> 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.
> .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.
> .flow_type_rss_offloads = 0x0,
> };
>
<...>
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,
> > };
> >
>
> <...>
>
@@ -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
Packet type parsing = Y
Basic stats = Y
Stats per queue = Y
@@ -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,
.tx_offload_capa = 0x0,
.flow_type_rss_offloads = 0x0,
};
+/**
+ * Check whether a specific offloading capability
+ * is supported by a sub_device.
+ *
+ * @return
+ * 0: all requested capabilities are supported by the sub_device
+ * positive value: This flag at least is not supported by the sub_device
+ */
+static int
+fs_port_offload_validate(struct rte_eth_dev *dev,
+ struct sub_device *sdev)
+{
+ struct rte_eth_dev_info infos = {0};
+ struct rte_eth_conf *cf;
+ uint32_t cap;
+
+ cf = &dev->data->dev_conf;
+ SUBOPS(sdev, dev_infos_get)(ETH(sdev), &infos);
+ /* RX capabilities */
+ cap = infos.rx_offload_capa;
+ if (cf->rxmode.hw_vlan_strip &&
+ ((cap & DEV_RX_OFFLOAD_VLAN_STRIP) == 0)) {
+ WARN("VLAN stripping offload requested but not supported by sub_device %d",
+ SUB_ID(sdev));
+ return DEV_RX_OFFLOAD_VLAN_STRIP;
+ }
+ if (cf->rxmode.hw_ip_checksum &&
+ ((cap & (DEV_RX_OFFLOAD_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_UDP_CKSUM |
+ DEV_RX_OFFLOAD_TCP_CKSUM)) !=
+ (DEV_RX_OFFLOAD_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_UDP_CKSUM |
+ DEV_RX_OFFLOAD_TCP_CKSUM))) {
+ WARN("IP checksum offload requested but not supported by sub_device %d",
+ SUB_ID(sdev));
+ return DEV_RX_OFFLOAD_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_UDP_CKSUM |
+ DEV_RX_OFFLOAD_TCP_CKSUM;
+ }
+ if (cf->rxmode.enable_lro &&
+ ((cap & DEV_RX_OFFLOAD_TCP_LRO) == 0)) {
+ WARN("TCP LRO offload requested but not supported by sub_device %d",
+ SUB_ID(sdev));
+ return DEV_RX_OFFLOAD_TCP_LRO;
+ }
+ if (cf->rxmode.hw_vlan_extend &&
+ ((cap & DEV_RX_OFFLOAD_QINQ_STRIP) == 0)) {
+ WARN("Stacked VLAN stripping offload requested but not supported by sub_device %d",
+ SUB_ID(sdev));
+ return DEV_RX_OFFLOAD_QINQ_STRIP;
+ }
+ /* TX capabilities */
+ /* Nothing to do, no tx capa supported */
+ return 0;
+}
+
+/*
+ * Disable the dev_conf flag related to an offload capability flag
+ * within an ethdev configuration.
+ */
+static int
+fs_port_disable_offload(struct rte_eth_conf *cf,
+ uint32_t ol_cap)
+{
+ switch (ol_cap) {
+ case DEV_RX_OFFLOAD_VLAN_STRIP:
+ INFO("Disabling VLAN stripping offload");
+ cf->rxmode.hw_vlan_strip = 0;
+ break;
+ case DEV_RX_OFFLOAD_IPV4_CKSUM:
+ case DEV_RX_OFFLOAD_UDP_CKSUM:
+ case DEV_RX_OFFLOAD_TCP_CKSUM:
+ case (DEV_RX_OFFLOAD_IPV4_CKSUM |
+ DEV_RX_OFFLOAD_UDP_CKSUM |
+ DEV_RX_OFFLOAD_TCP_CKSUM):
+ INFO("Disabling IP checksum offload");
+ cf->rxmode.hw_ip_checksum = 0;
+ break;
+ case DEV_RX_OFFLOAD_TCP_LRO:
+ INFO("Disabling TCP LRO offload");
+ cf->rxmode.enable_lro = 0;
+ break;
+ case DEV_RX_OFFLOAD_QINQ_STRIP:
+ INFO("Disabling stacked VLAN stripping offload");
+ cf->rxmode.hw_vlan_extend = 0;
+ break;
+ default:
+ DEBUG("Unable to disable offload capability: %" PRIx32,
+ ol_cap);
+ return -1;
+ }
+ return 0;
+}
+
static int
fs_dev_configure(struct rte_eth_dev *dev)
{
struct sub_device *sdev;
uint8_t i;
+ int capa_flag;
int ret;
FOREACH_SUBDEV(sdev, i, dev) {
if (sdev->state != DEV_PROBED)
continue;
+ DEBUG("Checking capabilities for sub_device %d", i);
+ while ((capa_flag = fs_port_offload_validate(dev, sdev))) {
+ /*
+ * Refuse to change configuration if multiple devices
+ * are present and we already have configured at least
+ * some of them.
+ */
+ if (PRIV(dev)->state >= DEV_ACTIVE &&
+ PRIV(dev)->subs_tail > 1) {
+ ERROR("device already configured, cannot fix live configuration");
+ return -1;
+ }
+ ret = fs_port_disable_offload(&dev->data->dev_conf,
+ capa_flag);
+ if (ret) {
+ ERROR("Unable to disable offload capability");
+ return ret;
+ }
+ }
+ }
+ FOREACH_SUBDEV(sdev, i, dev) {
+ if (sdev->state != DEV_PROBED)
+ continue;
DEBUG("Configuring sub-device %d", i);
ret = rte_eth_dev_configure(PORT_ID(sdev),
dev->data->nb_rx_queues,