[v1] net/ice: avoid the parsed devargs value being overwritten
Checks
Commit Message
If the default dev args 'proto_xtr' is not in the first position, it
will overwrite the parsed queue map value, so use an new variable to
save the default.
And enhance the error message printing to show the right information.
Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
drivers/net/ice/ice_ethdev.c | 44 +++++++++++++++++++++++++-----------
drivers/net/ice/ice_ethdev.h | 1 +
2 files changed, 32 insertions(+), 13 deletions(-)
Comments
On 10/09, Haiyue Wang wrote:
>If the default dev args 'proto_xtr' is not in the first position, it
>will overwrite the parsed queue map value, so use an new variable to
>save the default.
>
>And enhance the error message printing to show the right information.
>
>Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
>---
> drivers/net/ice/ice_ethdev.c | 44 +++++++++++++++++++++++++-----------
> drivers/net/ice/ice_ethdev.h | 1 +
> 2 files changed, 32 insertions(+), 13 deletions(-)
>
[snip]
>@@ -2126,6 +2142,8 @@ ice_dev_close(struct rte_eth_dev *dev)
> rte_free(hw->port_info);
> hw->port_info = NULL;
> ice_shutdown_all_ctrlq(hw);
>+ rte_free(pf->proto_xtr);
>+ pf->proto_xtr = NULL;
These two lines seems irrelevant to this patch, I noticed that the resource
free operation was contained in the original patch, and they are removed
unexpectedly by commit bd513ece3c40 ("net/ice: release port upon close"),
sorry for not catching it when reviewing.
Prefer one individual patch for the fix.
Thanks,
Xiaolong
>
> dev->dev_ops = NULL;
> dev->rx_pkt_burst = NULL;
>diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
>index 182c6f611..2fd98817b 100644
>--- a/drivers/net/ice/ice_ethdev.h
>+++ b/drivers/net/ice/ice_ethdev.h
>@@ -288,6 +288,7 @@ struct ice_pf {
> */
> struct ice_devargs {
> int safe_mode_support;
>+ uint8_t proto_xtr_dflt;
> uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
> };
>
>--
>2.17.1
>
> -----Original Message-----
> From: Ye, Xiaolong
> Sent: Monday, October 14, 2019 16:25
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v1] net/ice: avoid the parsed devargs value being overwritten
>
> On 10/09, Haiyue Wang wrote:
> >If the default dev args 'proto_xtr' is not in the first position, it
> >will overwrite the parsed queue map value, so use an new variable to
> >save the default.
> >
> >And enhance the error message printing to show the right information.
> >
> >Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> >---
> > drivers/net/ice/ice_ethdev.c | 44 +++++++++++++++++++++++++-----------
> > drivers/net/ice/ice_ethdev.h | 1 +
> > 2 files changed, 32 insertions(+), 13 deletions(-)
> >
>
> [snip]
>
> >@@ -2126,6 +2142,8 @@ ice_dev_close(struct rte_eth_dev *dev)
> > rte_free(hw->port_info);
> > hw->port_info = NULL;
> > ice_shutdown_all_ctrlq(hw);
> >+ rte_free(pf->proto_xtr);
> >+ pf->proto_xtr = NULL;
>
> These two lines seems irrelevant to this patch, I noticed that the resource
> free operation was contained in the original patch, and they are removed
> unexpectedly by commit bd513ece3c40 ("net/ice: release port upon close"),
> sorry for not catching it when reviewing.
>
> Prefer one individual patch for the fix.
>
Planed to send two patches, but a little lazy. :) Will do this in v2.
> Thanks,
> Xiaolong
>
> >
> > dev->dev_ops = NULL;
> > dev->rx_pkt_burst = NULL;
> >diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
> >index 182c6f611..2fd98817b 100644
> >--- a/drivers/net/ice/ice_ethdev.h
> >+++ b/drivers/net/ice/ice_ethdev.h
> >@@ -288,6 +288,7 @@ struct ice_pf {
> > */
> > struct ice_devargs {
> > int safe_mode_support;
> >+ uint8_t proto_xtr_dflt;
> > uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
> > };
> >
> >--
> >2.17.1
> >
@@ -428,8 +428,7 @@ parse_queue_proto_xtr(const char *queues, struct ice_devargs *devargs)
if (xtr_type < 0)
return -1;
- memset(devargs->proto_xtr, xtr_type,
- sizeof(devargs->proto_xtr));
+ devargs->proto_xtr_dflt = xtr_type;
return 0;
}
@@ -1369,12 +1368,36 @@ ice_interrupt_handler(void *param)
rte_intr_ack(dev->intr_handle);
}
+static void
+ice_init_proto_xtr(struct rte_eth_dev *dev)
+{
+ struct ice_adapter *ad =
+ ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
+ struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+ struct ice_hw *hw = ICE_PF_TO_HW(pf);
+ uint16_t i;
+
+ if (!ice_proto_xtr_support(hw)) {
+ PMD_DRV_LOG(NOTICE, "Protocol extraction is not supported");
+ return;
+ }
+
+ pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
+ if (unlikely(pf->proto_xtr == NULL)) {
+ PMD_DRV_LOG(ERR, "No memory for setting up protocol extraction table");
+ return;
+ }
+
+ for (i = 0; i < pf->lan_nb_qps; i++)
+ pf->proto_xtr[i] = ad->devargs.proto_xtr[i] != PROTO_XTR_NONE ?
+ ad->devargs.proto_xtr[i] :
+ ad->devargs.proto_xtr_dflt;
+}
+
/* Initialize SW parameters of PF */
static int
ice_pf_sw_init(struct rte_eth_dev *dev)
{
- struct ice_adapter *ad =
- ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
struct ice_pf *pf = ICE_DEV_PRIVATE_TO_PF(dev->data->dev_private);
struct ice_hw *hw = ICE_PF_TO_HW(pf);
@@ -1384,15 +1407,7 @@ ice_pf_sw_init(struct rte_eth_dev *dev)
pf->lan_nb_qps = pf->lan_nb_qp_max;
- if (ice_proto_xtr_support(hw))
- pf->proto_xtr = rte_zmalloc(NULL, pf->lan_nb_qps, 0);
-
- if (pf->proto_xtr != NULL)
- rte_memcpy(pf->proto_xtr, ad->devargs.proto_xtr,
- RTE_MIN((size_t)pf->lan_nb_qps,
- sizeof(ad->devargs.proto_xtr)));
- else
- PMD_DRV_LOG(NOTICE, "Protocol extraction is disabled");
+ ice_init_proto_xtr(dev);
return 0;
}
@@ -1805,6 +1820,7 @@ static int ice_parse_devargs(struct rte_eth_dev *dev)
return -EINVAL;
}
+ ad->devargs.proto_xtr_dflt = PROTO_XTR_NONE;
memset(ad->devargs.proto_xtr, PROTO_XTR_NONE,
sizeof(ad->devargs.proto_xtr));
@@ -2126,6 +2142,8 @@ ice_dev_close(struct rte_eth_dev *dev)
rte_free(hw->port_info);
hw->port_info = NULL;
ice_shutdown_all_ctrlq(hw);
+ rte_free(pf->proto_xtr);
+ pf->proto_xtr = NULL;
dev->dev_ops = NULL;
dev->rx_pkt_burst = NULL;
@@ -288,6 +288,7 @@ struct ice_pf {
*/
struct ice_devargs {
int safe_mode_support;
+ uint8_t proto_xtr_dflt;
uint8_t proto_xtr[ICE_MAX_QUEUE_NUM];
};