From: "Zhao1, Wei" <wei.zhao1@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>,
"Peng, Yuan" <yuan.peng@intel.com>,
"Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by default configuration
Date: Fri, 24 May 2019 01:55:21 +0000 [thread overview]
Message-ID: <A2573D2ACFCADC41BB3BE09C6DE313CA07EF5250@PGSMSX103.gar.corp.intel.com> (raw)
In-Reply-To: <000aa73c-fe6f-60d1-6060-5edf45e4349a@intel.com>
Hi, Ferruh
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Tuesday, May 21, 2019 11:43 PM
> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads overwrite by
> default configuration
>
> On 5/21/2019 2:30 AM, Zhao1, Wei wrote:
> > Hi, Ferruh
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh
> >> Sent: Monday, May 20, 2019 11:23 PM
> >> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >> Cc: stable@dpdk.org; Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo
> >> <wenzhuo.lu@intel.com>
> >> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads
> >> overwrite by default configuration
> >>
> >> On 5/14/2019 2:56 AM, Zhao1, Wei wrote:
> >>> Hi, Ferruh
> >>>
> >>>> -----Original Message-----
> >>>> From: Yigit, Ferruh
> >>>> Sent: Tuesday, May 14, 2019 12:36 AM
> >>>> To: Zhao1, Wei <wei.zhao1@intel.com>; dev@dpdk.org
> >>>> Cc: stable@dpdk.org; Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo
> >>>> <wenzhuo.lu@intel.com>
> >>>> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix offloads
> >>>> overwrite by default configuration
> >>>>
> >>>> On 5/9/2019 8:20 AM, Wei Zhao wrote:
> >>>>> There is an error in function rxtx_port_config(), which may
> >>>>> overwrite offloads configuration get from function
> >>>>> launch_args_parse() when run testpmd app. So rxtx_port_config()
> >>>>> should
> >> do "or" for port offloads.
> >>>>>
> >>>>> Fixes: d44f8a485f5d ("app/testpmd: enable per queue configure")
> >>>>> cc: stable@dpdk.org
> >>>>>
> >>>>> Signed-off-by: Wei Zhao <wei.zhao1@intel.com>
> >>>>> ---
> >>>>> app/test-pmd/testpmd.c | 5 +++++
> >>>>> 1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>>>> 6fbfd29..f0061d9 100644
> >>>>> --- a/app/test-pmd/testpmd.c
> >>>>> +++ b/app/test-pmd/testpmd.c
> >>>>> @@ -2809,9 +2809,12 @@ static void rxtx_port_config(struct
> >>>>> rte_port *port) {
> >>>>> uint16_t qid;
> >>>>> + uint64_t offloads;
> >>>>>
> >>>>> for (qid = 0; qid < nb_rxq; qid++) {
> >>>>> + offloads = port->rx_conf[qid].offloads;
> >>>>> port->rx_conf[qid] = port->dev_info.default_rxconf;
> >>>>> + port->rx_conf[qid].offloads |= offloads;
> >>>>
> >>>> OK to this changes as a fix for this release.
> >>>>
> >>>> But I think intention is, if no offload information is provided by
> >>>> user to use use the driver provided defaults, if user explicitly
> >>>> provided some values to use them, instead of OR these two.
> >>>>
> >>>> With this approach it is not possible to disable a driver default
> >>>> value, so it becomes mandatory offload instead of default offload values.
> >>>>
> >>>> Wei, what do you think, does it make sense?
> >>>
> >>>
> >>> I agree with you, but it is sure that the original code has offloads
> >>> overwrite
> >> issue.
> >>> What is your suggestion for code implement?
> >>> I find that Thomas has apply it, if you has other idea, maybe you
> >>> has to
> >> commit patch base to this patch.
> >>
> >> Hi Wei,
> >>
> >> Yes this needs to be incremental fix to existing code.
> >>
> >> Queue specific offload can be altered either by providing Rx/Tx
> >> offload as command line argument [1] (port configs set to each
> >> queues) or via testpmd commands [2].
> >> Does it make sense to set a global flag when one of above occurs and
> >> use default config only if it is not set?
> >
> > I AGREE with you to submit an incremental fix, and it make sense to
> > set a global flag when one of above occurs and use default config only if it is
> not set when implement code, but I do not have time to prepare such a patch
> by now, so maybe later or some else.
>
> I see, can you submit a public defect to record the issue, so it can be addressed
> later without forgotten?
Sure, but what is a public defect patch? Do you mean I need to update some doc? Can you give me a link as an example ?
>
> >
> >>
> >> [1]
> >> Tx
> >> tx-offloads
> >> Rx
> >> disable-crc-strip
> >> enable-lro
> >> enable-scatter
> >> enable-rx-cksum
> >> enable-rx-timestamp
> >> enable-hw-vlan
> >> enable-hw-vlan-filter
> >> enable-hw-vlan-strip
> >> enable-hw-vlan-extend
> >>
> >> [2]
> >> "port config <port_id> rx_offload ..."
> >> "port <port_id> rxq <queue_id> rx_offload ..."
> >> "port config <port_id> tx_offload ..."
> >> "port <port_id> txq <queue_id> tx_offload ..."
> >>
> >
next prev parent reply other threads:[~2019-05-24 1:58 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-09 7:20 [dpdk-dev] " Wei Zhao
2019-05-09 7:20 ` Wei Zhao
2019-05-13 3:30 ` Zhao1, Wei
2019-05-13 3:30 ` Zhao1, Wei
2019-05-13 15:08 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2019-05-13 15:08 ` Thomas Monjalon
2019-05-13 16:35 ` Ferruh Yigit
2019-05-13 16:35 ` Ferruh Yigit
2019-05-14 1:56 ` Zhao1, Wei
2019-05-14 1:56 ` Zhao1, Wei
2019-05-20 15:23 ` Ferruh Yigit
2019-05-21 1:30 ` Zhao1, Wei
2019-05-21 15:42 ` Ferruh Yigit
2019-05-24 1:55 ` Zhao1, Wei [this message]
2019-05-24 13:03 ` Ferruh Yigit
2019-06-10 7:27 ` Zhao1, Wei
2019-06-11 14:37 ` Ferruh Yigit
2019-06-12 0:57 ` Zhao1, Wei
2019-06-12 1:17 ` Zhao1, Wei
2019-06-14 15:42 ` Ferruh Yigit
2019-06-17 1:51 ` Zhao1, Wei
2019-06-17 12:59 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A2573D2ACFCADC41BB3BE09C6DE313CA07EF5250@PGSMSX103.gar.corp.intel.com \
--to=wei.zhao1@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=stable@dpdk.org \
--cc=wenzhuo.lu@intel.com \
--cc=yuan.peng@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).