On 11/19, Zhang, Qi Z wrote:
>
>
>> -----Original Message-----
>> From: Ye, Xiaolong <xiaolong...@intel.com>
>> Sent: Tuesday, November 19, 2019 1:53 PM
>> To: Zhang, Qi Z <qi.z.zh...@intel.com>
>> Cc: Yang, Qiming <qiming.y...@intel.com>; Cao, Yahui
>> <yahui....@intel.com>; Xing, Beilei <beilei.x...@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3] net/ice: add flow mark hint support
>>
>> On 11/18, Qi Zhang wrote:
>> >Since not all data paths support flow mark, the driver need
>>
>> s/need/needs
>>
>> >a hint from application to select the correct data path if flow mark is
>> >required. The patch introduce a devarg
>>
>> s/introduce/introduces
>>
>> >"flow-mark-support" as a workaround solution, since a standard way is
>> >still ongoing.
>> >
>> >Signed-off-by: Qi Zhang <qi.z.zh...@intel.com>
>> >---
>> >
>> >v3:
>> >- add "experimental notification" in ice.rst
>> >
>> >v2:
>> >- fix typo
>> >
>> > doc/guides/nics/ice.rst | 12 ++++++++++++
>> > drivers/net/ice/ice_ethdev.c | 11 ++++++++++-
>> > drivers/net/ice/ice_ethdev.h | 1 +
>> > drivers/net/ice/ice_rxtx_vec_common.h | 5 +++++
>> > 4 files changed, 28 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst index
>> >1a426438d..d1b80c2fa 100644
>> >--- a/doc/guides/nics/ice.rst
>> >+++ b/doc/guides/nics/ice.rst
>> >@@ -80,6 +80,18 @@ Runtime Config Options
>> >
>> > -w 80:00.0,pipeline-mode-support=1
>> >
>> >+- ``Flow Mark Support`` (default ``0``)
>> >+
>> >+ This is a hint to the driver to select the data path that support
>> >+ flow mark extraction
>>
>> s/support/supports
>>
>> >+ by default.
>> >+ NOTE: This is an experimental devarg, it will be removed when any of
>> >+ below conditions is ready.
>> >+ 1) all data path support flow mark (currently vPMD does not)
>>
>> s/path/paths
>>
>> >+ 2) a new offload like RTE_DEV_RX_OFFLOAD_FLOW_MARK be introduced
>> as a standard way to hint.
>> >+ Example::
>> >+
>> >+ -w 80:00.0,flow-mark-support=1
>> >+
>> > - ``Protocol extraction for per queue``
>> >
>> > Configure the RX queues to do protocol extraction into mbuf for
>> >protocol diff --git a/drivers/net/ice/ice_ethdev.c
>> >b/drivers/net/ice/ice_ethdev.c index abf00d404..9f2cb2f40 100644
>> >--- a/drivers/net/ice/ice_ethdev.c
>> >+++ b/drivers/net/ice/ice_ethdev.c
>> >@@ -23,11 +23,13 @@
>> > /* devargs */
>> > #define ICE_SAFE_MODE_SUPPORT_ARG "safe-mode-support"
>> > #define ICE_PIPELINE_MODE_SUPPORT_ARG "pipeline-mode-support"
>> >+#define ICE_FLOW_MARK_SUPPORT_ARG "flow-mark-support"
>> > #define ICE_PROTO_XTR_ARG "proto_xtr"
>> >
>> > static const char * const ice_valid_args[] = {
>> > ICE_SAFE_MODE_SUPPORT_ARG,
>> > ICE_PIPELINE_MODE_SUPPORT_ARG,
>> >+ ICE_FLOW_MARK_SUPPORT_ARG,
>> > ICE_PROTO_XTR_ARG,
>> > NULL
>> > };
>> >@@ -1987,6 +1989,12 @@ static int ice_parse_devargs(struct rte_eth_dev
>> >*dev)
>> >
>> > ret = rte_kvargs_process(kvlist, ICE_PIPELINE_MODE_SUPPORT_ARG,
>> > &parse_bool, &ad->devargs.pipe_mode_support);
>> >+ if (ret)
>> >+ goto bail;
>> >+
>> >+ ret = rte_kvargs_process(kvlist, ICE_FLOW_MARK_SUPPORT_ARG,
>> >+ &parse_bool, &ad->devargs.flow_mark_support);
>>
>> Do we need error handling here?
>>
>> >+ printf("flow_mark = %d\n", ad->devargs.flow_mark_support);
>>
>> Use PMD_INIT_LOG instead, and in order to provide more informative
>> message, what about we directly say "flow_mark is 0, vector path won't be
>> selected." or something similar according to the value of flow_makr_support.
>>
>
>This is a mistake forgot to remove the debug code.
Ok, I think it's still meaningful to print some hints in
ice_rx_vec_dev_check_default
when it return due to flow_mark_support is set.
Thanks,
Xiaolong
