| Message ID | 20181109201327.30587-2-ktraynor@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | Update to DPDK 18.11-rc2 | expand |
> DEV_RX_OFFLOAD_CRC_STRIP has been removed from DPDK 18.11. > DEV_RX_OFFLOAD_KEEP_CRC can now be used to keep the CRC. This doesn't > change any behaviour in OVS, just updates to use the correct flags. > > Signed-off-by: Kevin Traynor <ktraynor@redhat.com> > --- > lib/netdev-dpdk.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > 9694e0710..10c4879a1 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -930,6 +930,6 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int > n_rxq, int n_txq) > } > > - if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > - conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > + if (!(dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) { > + conf.rxmode.offloads |= DEV_RX_OFFLOAD_KEEP_CRC; > } Hi Kevin, Thanks for this series. Technically the behavior from OVS does change here as previously if NETDEV_RX_HW_CRC_STRIP wasn't supported we wouldn't set DEV_RX_OFFLOAD_KEEP_CRC. For vdevs it could be the case that this isn't supported. I spotted this testing the net_null pmd, it now fails to init. It's a corner case for sure but probably should be checked that support is there before explicitly setting it. Thanks Ian
> -----Original Message----- > From: Stokes, Ian [mailto:ian.stokes@intel.com] > Sent: Wednesday, November 14, 2018 3:16 PM > To: Kevin Traynor <ktraynor@redhat.com>; dev@openvswitch.org; Ophir > Munk <ophirmu@mellanox.com>; i.maximets@samsung.com > Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com> > Subject: RE: [RFC v2 dpdk-latest 1/2] netdev-dpdk: Update for DPDK CRC > strip flags change. > > > - if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > > - conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > > + if (!(dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) { > > + conf.rxmode.offloads |= DEV_RX_OFFLOAD_KEEP_CRC; > > } > > Hi Kevin, > > Thanks for this series. Technically the behavior from OVS does change here > as previously if NETDEV_RX_HW_CRC_STRIP wasn't supported we wouldn't > set DEV_RX_OFFLOAD_KEEP_CRC. For vdevs it could be the case that this > isn't supported. > > I spotted this testing the net_null pmd, it now fails to init. It's a corner case > for sure but probably should be checked that support is there before > explicitly setting it. > This case is handled in https://patchwork.ozlabs.org/patch/997879/ > Thanks > Ian
On 11/15/2018 04:40 PM, Ophir Munk wrote: > > >> -----Original Message----- >> From: Stokes, Ian [mailto:ian.stokes@intel.com] >> Sent: Wednesday, November 14, 2018 3:16 PM >> To: Kevin Traynor <ktraynor@redhat.com>; dev@openvswitch.org; Ophir >> Munk <ophirmu@mellanox.com>; i.maximets@samsung.com >> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com> >> Subject: RE: [RFC v2 dpdk-latest 1/2] netdev-dpdk: Update for DPDK CRC >> strip flags change. >> >>> - if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >>> - conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >>> + if (!(dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) { >>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_KEEP_CRC; >>> } >> >> Hi Kevin, >> >> Thanks for this series. Technically the behavior from OVS does change here >> as previously if NETDEV_RX_HW_CRC_STRIP wasn't supported we wouldn't >> set DEV_RX_OFFLOAD_KEEP_CRC. For vdevs it could be the case that this >> isn't supported. >> Hi Ian, previously there was no check to see if STRIP/KEEP was supported but DPDK wasn't strict about it, so it worked. Now DPDK cares, so the check must be added. Will send v3 with check. >> I spotted this testing the net_null pmd, it now fails to init. It's a corner case >> for sure but probably should be checked that support is there before >> explicitly setting it. >> > > This case is handled in https://patchwork.ozlabs.org/patch/997879/ > yep, a check needed to be added against device capability, as for example net_null reports not supporting offloads. thanks, Kevin. >> Thanks >> Ian >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 9694e0710..10c4879a1 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -930,6 +930,6 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) } - if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { - conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; + if (!(dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP)) { + conf.rxmode.offloads |= DEV_RX_OFFLOAD_KEEP_CRC; }
DEV_RX_OFFLOAD_CRC_STRIP has been removed from DPDK 18.11. DEV_RX_OFFLOAD_KEEP_CRC can now be used to keep the CRC. This doesn't change any behaviour in OVS, just updates to use the correct flags. Signed-off-by: Kevin Traynor <ktraynor@redhat.com> --- lib/netdev-dpdk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)