subject:"\[ovs\-dev\] \[PATCH 1\/2\] netdev\-dpdk\: Fix Rx checksum reconfigure."

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

2017-04-25 Thread Chandran, Sugesh



Regards
_Sugesh

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Tuesday, April 25, 2017 6:40 PM
> To: Chandran, Sugesh ; d...@openvswitch.org
> Subject: Re: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
> 
> On 04/10/2017 04:31 PM, Chandran, Sugesh wrote:
> > Thank you for the patch,
> > Please see the comment below.
> >
> > Regards
> > _Sugesh
> >
> >> -Original Message-
> >> From: Kevin Traynor [mailto:ktray...@redhat.com]
> >> Sent: Tuesday, April 4, 2017 6:35 AM
> >> To: d...@openvswitch.org
> >> Cc: Kevin Traynor ; Chandran, Sugesh
> >> 
> >> Subject: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
> >>
> >> Rx checksum offload is enabled by default where available, with
> >> reconfiguration through OVSDB options:rx-checksum-offload.
> >> However, setting rx-checksum-offload does not result in a
> >> reconfiguration of the NIC.
> >>
> >> Fix that by checking if the requested port config features (e.g. rx
> >> checksum
> >> offload) are currently applied on the NIC and if not, perform a
> >> reconfiguration.
> >>
> >> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading
> >> feature on DPDK physical ports.")
> >> Cc: Sugesh Chandran 
> >> Signed-off-by: Kevin Traynor 
> >> ---
> >>  lib/netdev-dpdk.c | 14 +-
> >>  1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> ddc651b..d5a9800
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -374,4 +374,5 @@ struct netdev_dpdk {
> >>  int requested_rxq_size;
> >>  int requested_txq_size;
> >> +uint32_t requested_hwol;
> >>
> >>  /* Number of rx/tx descriptors for physical devices */ @@ -647,5
> >> +648,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int
> >> n_rxq, int
> >> n_txq)
> >>  conf.rxmode.max_rx_pkt_len = 0;
> >>  }
> >> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> >> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
> >>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >>  /* A device may report more queues than it makes available (this
> >> has @@
> >> -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk
> *dev,
> >> int n_rxq, int n_txq)
> >>  dev->up.n_rxq = n_rxq;
> >>  dev->up.n_txq = n_txq;
> >> +dev->hw_ol_features = dev->requested_hwol;
> >>
> >>  return 0;
> >> @@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct
> >> netdev_dpdk *dev)
> >>   DEV_RX_OFFLOAD_IPV4_CKSUM;
> >>  rte_eth_dev_info_get(dev->port_id, &info);
> >> -rx_csum_ol_flag = (dev->hw_ol_features &
> >> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >> +rx_csum_ol_flag = (dev->requested_hwol &
> >> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> >>
> >>  if (rx_csum_ol_flag &&
> >> @@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct
> >> netdev_dpdk *dev)
> >>  VLOG_WARN_ONCE("Rx checksum offload is not supported on
> >> device %d",
> >> dev->port_id);
> >> -dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> >> +dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> >>  return;
> >>  }
> >> @@ -871,4 +873,5 @@ common_construct(struct netdev *netdev,
> unsigned
> >> int port_no,
> >>  /* Initilize the hardware offload flags to 0 */
> >>  dev->hw_ol_features = 0;
> >> +dev->requested_hwol = 0;
> >>
> >>  dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1259,5 +1262,5
> @@
> >> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
> >>  != 0;
> >>  if (temp_flag != rx_chksm_ofld) {
> >> -dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> >> +dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> >>  dpdk_eth_checksum_offload_configure(dev);
> >>  }
> >> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev
> *netdev)
> >>  && dev->rxq_size == dev->requested_rxq_size
> >>  && dev->txq_size == dev->requested_txq_size
> >> -&& dev->socket_id == dev->requested_socket_id) {
> >> +&& dev->socket_id == dev->requested_socket_id
> >> +&& dev->hw_ol_features == dev->requested_hwol) {
> >>  /* Reconfiguration is unnecessary */
> > [Sugesh] The patch fixes the issue however I believe these checks has to
> be moved to the corresponding functions than here. May be that looks
> cleaner.  Any configuration change that happen to a netdev validated at the
> corresponding functions than in a common reconfigure. That can avoid the
> need of 2 struct element for every param.
> > I verified the patch and now it reloads the NIC configuration when there is
> a change.
> 
> Hi Sugesh, Not sure I totally follow you, I think this fixes the issue close 
> to the
> original patch and the current reconfigure approach. Are you suggesting that
> the reconfigure code s

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

2017-04-25 Thread Kevin Traynor

On 04/10/2017 04:31 PM, Chandran, Sugesh wrote:
> Thank you for the patch,
> Please see the comment below.
> 
> Regards
> _Sugesh
> 
>> -Original Message-
>> From: Kevin Traynor [mailto:ktray...@redhat.com]
>> Sent: Tuesday, April 4, 2017 6:35 AM
>> To: d...@openvswitch.org
>> Cc: Kevin Traynor ; Chandran, Sugesh
>> 
>> Subject: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
>>
>> Rx checksum offload is enabled by default where available, with
>> reconfiguration through OVSDB options:rx-checksum-offload.
>> However, setting rx-checksum-offload does not result in a reconfiguration of
>> the NIC.
>>
>> Fix that by checking if the requested port config features (e.g. rx checksum
>> offload) are currently applied on the NIC and if not, perform a
>> reconfiguration.
>>
>> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature
>> on DPDK physical ports.")
>> Cc: Sugesh Chandran 
>> Signed-off-by: Kevin Traynor 
>> ---
>>  lib/netdev-dpdk.c | 14 +-
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ddc651b..d5a9800
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -374,4 +374,5 @@ struct netdev_dpdk {
>>  int requested_rxq_size;
>>  int requested_txq_size;
>> +uint32_t requested_hwol;
>>
>>  /* Number of rx/tx descriptors for physical devices */ @@ -647,5 +648,5
>> @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int
>> n_txq)
>>  conf.rxmode.max_rx_pkt_len = 0;
>>  }
>> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
>>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>  /* A device may report more queues than it makes available (this has @@
>> -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
>> int n_rxq, int n_txq)
>>  dev->up.n_rxq = n_rxq;
>>  dev->up.n_txq = n_txq;
>> +dev->hw_ol_features = dev->requested_hwol;
>>
>>  return 0;
>> @@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct
>> netdev_dpdk *dev)
>>   DEV_RX_OFFLOAD_IPV4_CKSUM;
>>  rte_eth_dev_info_get(dev->port_id, &info);
>> -rx_csum_ol_flag = (dev->hw_ol_features &
>> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>> +rx_csum_ol_flag = (dev->requested_hwol &
>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>>
>>  if (rx_csum_ol_flag &&
>> @@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct
>> netdev_dpdk *dev)
>>  VLOG_WARN_ONCE("Rx checksum offload is not supported on device
>> %d",
>> dev->port_id);
>> -dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>> +dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>>  return;
>>  }
>> @@ -871,4 +873,5 @@ common_construct(struct netdev *netdev, unsigned
>> int port_no,
>>  /* Initilize the hardware offload flags to 0 */
>>  dev->hw_ol_features = 0;
>> +dev->requested_hwol = 0;
>>
>>  dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1259,5 +1262,5 @@
>> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>>  != 0;
>>  if (temp_flag != rx_chksm_ofld) {
>> -dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
>> +dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
>>  dpdk_eth_checksum_offload_configure(dev);
>>  }
>> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>>  && dev->rxq_size == dev->requested_rxq_size
>>  && dev->txq_size == dev->requested_txq_size
>> -&& dev->socket_id == dev->requested_socket_id) {
>> +&& dev->socket_id == dev->requested_socket_id
>> +&& dev->hw_ol_features == dev->requested_hwol) {
>>  /* Reconfiguration is unnecessary */
> [Sugesh] The patch fixes the issue however I believe these checks has to be 
> moved to the corresponding functions than here. May be that looks cleaner.  
> Any configuration change that happen to a netdev validated at the 
> corresponding functions than in a common reconfigure. That can avoid the need 
> of 2 struct element for every param.
> I verified the patch and now it reloads the NIC configuration when there is a 
> change.

Hi Sugesh, Not sure I totally follow you, I think this fixes the issue
close to the original patch and the current reconfigure approach. Are
you suggesting that the reconfigure code should be re-written to remove
the requested_*'s before this bug fix? or a different fix for this
specific issue? Maybe you could add comment at the LOC you think should
change so I understand.

thanks,
Kevin.

> 
> 
> 
>>
>> --
>> 1.8.3.1
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

2017-04-10 Thread Chandran, Sugesh

Thank you for the patch,
Please see the comment below.

Regards
_Sugesh

> -Original Message-
> From: Kevin Traynor [mailto:ktray...@redhat.com]
> Sent: Tuesday, April 4, 2017 6:35 AM
> To: d...@openvswitch.org
> Cc: Kevin Traynor ; Chandran, Sugesh
> 
> Subject: [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.
> 
> Rx checksum offload is enabled by default where available, with
> reconfiguration through OVSDB options:rx-checksum-offload.
> However, setting rx-checksum-offload does not result in a reconfiguration of
> the NIC.
> 
> Fix that by checking if the requested port config features (e.g. rx checksum
> offload) are currently applied on the NIC and if not, perform a
> reconfiguration.
> 
> Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature
> on DPDK physical ports.")
> Cc: Sugesh Chandran 
> Signed-off-by: Kevin Traynor 
> ---
>  lib/netdev-dpdk.c | 14 +-
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index ddc651b..d5a9800
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -374,4 +374,5 @@ struct netdev_dpdk {
>  int requested_rxq_size;
>  int requested_txq_size;
> +uint32_t requested_hwol;
> 
>  /* Number of rx/tx descriptors for physical devices */ @@ -647,5 +648,5
> @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int n_rxq, int
> n_txq)
>  conf.rxmode.max_rx_pkt_len = 0;
>  }
> -conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
> +conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
>NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
>  /* A device may report more queues than it makes available (this has @@
> -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev,
> int n_rxq, int n_txq)
>  dev->up.n_rxq = n_rxq;
>  dev->up.n_txq = n_txq;
> +dev->hw_ol_features = dev->requested_hwol;
> 
>  return 0;
> @@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct
> netdev_dpdk *dev)
>   DEV_RX_OFFLOAD_IPV4_CKSUM;
>  rte_eth_dev_info_get(dev->port_id, &info);
> -rx_csum_ol_flag = (dev->hw_ol_features &
> NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> +rx_csum_ol_flag = (dev->requested_hwol &
> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
> 
>  if (rx_csum_ol_flag &&
> @@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct
> netdev_dpdk *dev)
>  VLOG_WARN_ONCE("Rx checksum offload is not supported on device
> %d",
> dev->port_id);
> -dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
> +dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
>  return;
>  }
> @@ -871,4 +873,5 @@ common_construct(struct netdev *netdev, unsigned
> int port_no,
>  /* Initilize the hardware offload flags to 0 */
>  dev->hw_ol_features = 0;
> +dev->requested_hwol = 0;
> 
>  dev->flags = NETDEV_UP | NETDEV_PROMISC; @@ -1259,5 +1262,5 @@
> netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args,
>  != 0;
>  if (temp_flag != rx_chksm_ofld) {
> -dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
> +dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
>  dpdk_eth_checksum_offload_configure(dev);
>  }
> @@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
>  && dev->rxq_size == dev->requested_rxq_size
>  && dev->txq_size == dev->requested_txq_size
> -&& dev->socket_id == dev->requested_socket_id) {
> +&& dev->socket_id == dev->requested_socket_id
> +&& dev->hw_ol_features == dev->requested_hwol) {
>  /* Reconfiguration is unnecessary */
[Sugesh] The patch fixes the issue however I believe these checks has to be 
moved to the corresponding functions than here. May be that looks cleaner.  Any 
configuration change that happen to a netdev validated at the corresponding 
functions than in a common reconfigure. That can avoid the need of 2 struct 
element for every param.
I verified the patch and now it reloads the NIC configuration when there is a 
change.



> 
> --
> 1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

2017-04-04 Thread Kevin Traynor

Rx checksum offload is enabled by default where available, with
reconfiguration through OVSDB options:rx-checksum-offload.
However, setting rx-checksum-offload does not result in a
reconfiguration of the NIC.

Fix that by checking if the requested port config features
(e.g. rx checksum offload) are currently applied on the NIC and if
not, perform a reconfiguration.

Fixes: 1a2bb11817a4 ("netdev-dpdk: Enable Rx checksum offloading feature on 
DPDK physical ports.")
Cc: Sugesh Chandran 
Signed-off-by: Kevin Traynor 
---
 lib/netdev-dpdk.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ddc651b..d5a9800 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -374,4 +374,5 @@ struct netdev_dpdk {
 int requested_rxq_size;
 int requested_txq_size;
+uint32_t requested_hwol;
 
 /* Number of rx/tx descriptors for physical devices */
@@ -647,5 +648,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 conf.rxmode.max_rx_pkt_len = 0;
 }
-conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
+conf.rxmode.hw_ip_checksum = (dev->requested_hwol &
   NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
 /* A device may report more queues than it makes available (this has
@@ -701,4 +702,5 @@ dpdk_eth_dev_queue_setup(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 dev->up.n_rxq = n_rxq;
 dev->up.n_txq = n_txq;
+dev->hw_ol_features = dev->requested_hwol;
 
 return 0;
@@ -718,5 +720,5 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
  DEV_RX_OFFLOAD_IPV4_CKSUM;
 rte_eth_dev_info_get(dev->port_id, &info);
-rx_csum_ol_flag = (dev->hw_ol_features & NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
+rx_csum_ol_flag = (dev->requested_hwol & NETDEV_RX_CHECKSUM_OFFLOAD) != 0;
 
 if (rx_csum_ol_flag &&
@@ -725,5 +727,5 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev)
 VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d",
dev->port_id);
-dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
+dev->requested_hwol &= ~NETDEV_RX_CHECKSUM_OFFLOAD;
 return;
 }
@@ -871,4 +873,5 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
+dev->requested_hwol = 0;
 
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
@@ -1259,5 +1262,5 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 != 0;
 if (temp_flag != rx_chksm_ofld) {
-dev->hw_ol_features ^= NETDEV_RX_CHECKSUM_OFFLOAD;
+dev->requested_hwol ^= NETDEV_RX_CHECKSUM_OFFLOAD;
 dpdk_eth_checksum_offload_configure(dev);
 }
@@ -3124,5 +3127,6 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
 && dev->rxq_size == dev->requested_rxq_size
 && dev->txq_size == dev->requested_txq_size
-&& dev->socket_id == dev->requested_socket_id) {
+&& dev->socket_id == dev->requested_socket_id
+&& dev->hw_ol_features == dev->requested_hwol) {
 /* Reconfiguration is unnecessary */
 
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

Re: [ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

[ovs-dev] [PATCH 1/2] netdev-dpdk: Fix Rx checksum reconfigure.

4 matches

Site Navigation

Mail list logo

Footer information