From: "Zhang, Helin" <helin.zhang@intel.com>
To: "Topel, Bjorn" <bjorn.topel@intel.com>,
David Marchand <david.marchand@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Wu, Jingjing" <jingjing.wu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF hosts
Date: Tue, 19 Apr 2016 02:31:58 +0000 [thread overview]
Message-ID: <F35DEAC7BCE34641BA9FAC6BCA4A12E70AA1051C@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <6B5F13689D63284BBB0B66A5B315C82DBAAD8C@IRSMSX103.ger.corp.intel.com>
> -----Original Message-----
> From: Topel, Bjorn
> Sent: Tuesday, April 19, 2016 2:47 AM
> To: Zhang, Helin <helin.zhang@intel.com>; David Marchand
> <david.marchand@6wind.com>
> Cc: dev@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] i40evf: Ignore disabled HW CRC strip for Linux PF
> hosts
>
> >> Not sure this is the right way to handle it. The driver should
> >> return an error rather than silently discard what the application
> >> asked.
> >
> > I also think it should return an error with checking if the host is
> > kernel driver, and crc strip is disabled in VF. Thank you David!
>
> Thanks for reviewing my patch, Helin and David.
>
> I agree that it's subtle to ignore the error, and just log the error. This is how
> ixgbevf behaves (refer to ixgbevf_dev_configure), so I figured that i40evf should
> behave analogous.
>
> I'll submit a v2 of the patch that returns an EINVAL and logs the failure.
>
> Would it make sense to change the ixgbevf_dev_configure as well, in a separate
> patch?
Yes, I agree with you that ixgbe and i40e should be consistent. Thank you!
/Helin
>
>
> >> ---------------------------------------------------------------------
> >> -
> >> Intel Sweden AB
> >> Registered Office: Isafjordsgatan 30B, 164 40 Kista, Stockholm,
> >> Sweden Registration Number: 556189-6027
> >>
> >> This e-mail and any attachments may contain confidential material for
> >> the sole use of the intended recipient(s). Any review or distribution
> >> by others is strictly prohibited. If you are not the intended
> >> recipient, please contact the sender and delete all copies.
> >
> > Please, remove this.
>
> Noted. Will make sure to fix that for future revisions. Thanks!
>
>
> Björn
next prev parent reply other threads:[~2016-04-19 2:32 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-14 14:58 Björn Töpel
2016-04-15 7:40 ` David Marchand
2016-04-15 8:07 ` Zhang, Helin
2016-04-18 18:47 ` Topel, Bjorn
2016-04-19 2:31 ` Zhang, Helin [this message]
2016-04-21 15:02 ` [dpdk-dev] [PATCH v2] i40evf: Report error if HW CRC strip is disabled " Björn Töpel
2016-04-22 1:52 ` Zhang, Helin
2016-04-22 4:54 ` Topel, Bjorn
2016-04-22 5:07 ` Zhang, Helin
2016-04-22 5:17 ` Topel, Bjorn
2016-04-22 5:18 ` Zhang, Helin
2016-04-22 5:39 ` [dpdk-dev] [PATCH v3] i40evf: Report error if HW CRC strip is disabled for non-DPDK " Björn Töpel
2016-04-29 13:42 ` Zhang, Helin
2016-05-03 10:55 ` Bruce Richardson
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=F35DEAC7BCE34641BA9FAC6BCA4A12E70AA1051C@SHSMSX103.ccr.corp.intel.com \
--to=helin.zhang@intel.com \
--cc=bjorn.topel@intel.com \
--cc=david.marchand@6wind.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@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).