| Message ID | 20220606153535.3574930-1-xuejun.zhang@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [net,v1] iavf: Fix max_rate limiting | expand |
On 6/6/2022 8:35 AM, Jun Zhang wrote: > Fix max_rate option in tc, check for proper quanta boundaries. > Check for minimum value provided and if it fits expected 50Mbps > quanta. > > Without this patch, iavf could send settings for max_rate limiting > that would be accepted from by PF even the max_rate option is less > than expected 50Mbps quanta. It results in no rate limiting > on traffic as rate limiting will be floored to 0. > > Example: > tc qdisc add dev $vf root mqprio num_tc 3 map 0 2 1 queues \ > 2@0 2@2 2@4 hw 1 mode channel shaper bw_rlimit \ > max_rate 50Mbps 500Mbps 500Mbps > > Should limit TC0 to circa 50 Mbps > > tc qdisc add dev $vf root mqprio num_tc 3 map 0 2 1 queues \ > 2@0 2@2 2@4 hw 1 mode channel shaper bw_rlimit \ > max_rate 0Mbps 100Kbit 500Mbps > > Should return error > > Fixes: d5b33d024496 ("i40evf: add ndo_setup_tc callback to i40evf") > Signed-off-by: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com> > Signed-off-by: Jun Zhang <xuejun.zhang@intel.com> Who is the author for this? If it's Jun, your name needs to be first. If it's Przemyslaw, the author needs to be fixed on the patch. > --- > drivers/net/ethernet/intel/iavf/iavf.h | 1 + > drivers/net/ethernet/intel/iavf/iavf_main.c | 24 +++++++++++++++++++-- > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h > index fda1198d2c00..0bd516ecebef 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf.h > +++ b/drivers/net/ethernet/intel/iavf/iavf.h > @@ -93,6 +93,7 @@ struct iavf_vsi { > #define IAVF_HKEY_ARRAY_SIZE ((IAVF_VFQF_HKEY_MAX_INDEX + 1) * 4) > #define IAVF_HLUT_ARRAY_SIZE ((IAVF_VFQF_HLUT_MAX_INDEX + 1) * 4) > #define IAVF_MBPS_DIVISOR 125000 /* divisor to convert to Mbps */ > +#define IAVF_MBPS_QUANTA 50 > > #define IAVF_VIRTCHNL_VF_RESOURCE_SIZE (sizeof(struct virtchnl_vf_resource) + \ > (IAVF_MAX_VF_VSI * \ > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c > index 57c51a15bcbc..27a39d86268d 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > @@ -3411,6 +3411,7 @@ static int iavf_validate_ch_config(struct iavf_adapter *adapter, > u64 total_max_rate = 0; > int i, num_qps = 0; > u64 tx_rate = 0; > + u32 tx_rate_rem = 0; nit: Please arrange as RCT. > int ret = 0; > > if (mqprio_qopt->qopt.num_tc > IAVF_MAX_TRAFFIC_CLASS || > @@ -3423,12 +3424,31 @@ static int iavf_validate_ch_config(struct iavf_adapter *adapter, > return -EINVAL; > if (mqprio_qopt->min_rate[i]) { > dev_err(&adapter->pdev->dev, > - "Invalid min tx rate (greater than 0) specified\n"); > + "Invalid min tx rate (greater than 0) specified for TC%d\n", i); > return -EINVAL; > } > - /*convert to Mbps */ > + > + /* convert to Mbps */ > tx_rate = div_u64(mqprio_qopt->max_rate[i], > IAVF_MBPS_DIVISOR); > + > + if (mqprio_qopt->max_rate[i] && > + tx_rate < IAVF_MBPS_QUANTA) { > + dev_err(&adapter->pdev->dev, > + "Invalid max tx rate for TC%d, minimum %dMbps\n", > + i, IAVF_MBPS_QUANTA); > + return -EINVAL; > + } > + > + (void)div_u64_rem(tx_rate, IAVF_MBPS_QUANTA, &tx_rate_rem); > + > + if (tx_rate_rem != 0) { I believe the preference is if(tx_rate_rem) {... > + dev_err(&adapter->pdev->dev, > + "Invalid max tx rate for TC%d, not divisible by %d\n", > + i, IAVF_MBPS_QUANTA); > + return -EINVAL; > + } > + > total_max_rate += tx_rate; > num_qps += mqprio_qopt->qopt.count[i]; > }
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index fda1198d2c00..0bd516ecebef 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -93,6 +93,7 @@ struct iavf_vsi { #define IAVF_HKEY_ARRAY_SIZE ((IAVF_VFQF_HKEY_MAX_INDEX + 1) * 4) #define IAVF_HLUT_ARRAY_SIZE ((IAVF_VFQF_HLUT_MAX_INDEX + 1) * 4) #define IAVF_MBPS_DIVISOR 125000 /* divisor to convert to Mbps */ +#define IAVF_MBPS_QUANTA 50 #define IAVF_VIRTCHNL_VF_RESOURCE_SIZE (sizeof(struct virtchnl_vf_resource) + \ (IAVF_MAX_VF_VSI * \ diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 57c51a15bcbc..27a39d86268d 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -3411,6 +3411,7 @@ static int iavf_validate_ch_config(struct iavf_adapter *adapter, u64 total_max_rate = 0; int i, num_qps = 0; u64 tx_rate = 0; + u32 tx_rate_rem = 0; int ret = 0; if (mqprio_qopt->qopt.num_tc > IAVF_MAX_TRAFFIC_CLASS || @@ -3423,12 +3424,31 @@ static int iavf_validate_ch_config(struct iavf_adapter *adapter, return -EINVAL; if (mqprio_qopt->min_rate[i]) { dev_err(&adapter->pdev->dev, - "Invalid min tx rate (greater than 0) specified\n"); + "Invalid min tx rate (greater than 0) specified for TC%d\n", i); return -EINVAL; } - /*convert to Mbps */ + + /* convert to Mbps */ tx_rate = div_u64(mqprio_qopt->max_rate[i], IAVF_MBPS_DIVISOR); + + if (mqprio_qopt->max_rate[i] && + tx_rate < IAVF_MBPS_QUANTA) { + dev_err(&adapter->pdev->dev, + "Invalid max tx rate for TC%d, minimum %dMbps\n", + i, IAVF_MBPS_QUANTA); + return -EINVAL; + } + + (void)div_u64_rem(tx_rate, IAVF_MBPS_QUANTA, &tx_rate_rem); + + if (tx_rate_rem != 0) { + dev_err(&adapter->pdev->dev, + "Invalid max tx rate for TC%d, not divisible by %d\n", + i, IAVF_MBPS_QUANTA); + return -EINVAL; + } + total_max_rate += tx_rate; num_qps += mqprio_qopt->qopt.count[i]; }
Fix max_rate option in tc, check for proper quanta boundaries. Check for minimum value provided and if it fits expected 50Mbps quanta. Without this patch, iavf could send settings for max_rate limiting that would be accepted from by PF even the max_rate option is less than expected 50Mbps quanta. It results in no rate limiting on traffic as rate limiting will be floored to 0. Example: tc qdisc add dev $vf root mqprio num_tc 3 map 0 2 1 queues \ 2@0 2@2 2@4 hw 1 mode channel shaper bw_rlimit \ max_rate 50Mbps 500Mbps 500Mbps Should limit TC0 to circa 50 Mbps tc qdisc add dev $vf root mqprio num_tc 3 map 0 2 1 queues \ 2@0 2@2 2@4 hw 1 mode channel shaper bw_rlimit \ max_rate 0Mbps 100Kbit 500Mbps Should return error Fixes: d5b33d024496 ("i40evf: add ndo_setup_tc callback to i40evf") Signed-off-by: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com> Signed-off-by: Jun Zhang <xuejun.zhang@intel.com> --- drivers/net/ethernet/intel/iavf/iavf.h | 1 + drivers/net/ethernet/intel/iavf/iavf_main.c | 24 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-)