From: Vlad Zolotarov <vladz@cloudius-systems•com>
To: Alexander Duyck <alexander.h.duyck@redhat•com>,
"Tantilov, Emil S" <emil.s.tantilov@intel•com>,
"netdev@vger•kernel.org" <netdev@vger•kernel.org>
Cc: "Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel•com>,
"avi@cloudius-systems•com" <avi@cloudius-systems•com>,
"gleb@cloudius-systems•com" <gleb@cloudius-systems•com>,
"Skidmore, Donald C" <donald.c.skidmore@intel•com>
Subject: Re: [PATCH net-next v6 4/7] ixgbevf: Add a RETA query code
Date: Thu, 26 Mar 2015 20:10:45 +0200 [thread overview]
Message-ID: <55144BA5.5040507@cloudius-systems.com> (raw)
In-Reply-To: <551322F2.5020101@redhat.com>
On 03/25/15 23:04, Alexander Duyck wrote:
>
> On 03/25/2015 01:17 PM, Vlad Zolotarov wrote:
>>
>>
>> On 03/25/15 20:35, Tantilov, Emil S wrote:
>>>> -----Original Message-----
>>>> From: Vlad Zolotarov [mailto:vladz@cloudius-systems•com]
>>>> Sent: Wednesday, March 25, 2015 2:28 AM
>>>> Subject: Re: [PATCH net-next v6 4/7] ixgbevf: Add a RETA query code
>>> <snip>
>>>
>>>>> Have you tested what happens if you run:
>>>>>
>>>>> while true
>>>>> do
>>>>> ethtool --show-rxfh-indir ethX
>>>>> done
>>>>>
>>>>> in the background while passing traffic through the VF?
>>>> I understand your concerns but let's start with clarifying a few
>>>> things.
>>>> First, VF driver is by definition not trusted. If it (or its user)
>>>> decides to do anything malicious (like u proposed above) that would
>>>> eventually hurt (only this) VF's performance - nobody should care.
>>>> However the right question here would be: "How the above use case may
>>>> hurt the corresponding PF or other VFs' performance?" And since the
>>>> mailbox operation involves quite a few MMIO writes and reads this may
>>>> slow the PF quite a bit and this may be a problem that should be taken
>>>> care of. However it wasn't my patch series that have introduced it.
>>>> The
>>>> same problem would arise if Guest would change VF's MAC address in a
>>>> tight loop like above. Namely any VF slow path operation that would
>>>> eventually cause the VF-PF channel transaction may be used to
>>>> create an
>>>> attack on a PF.
>>> There are operations that can be disruptive to the VF I am not
>>> arguing that,
>>> the issue introduced by these patches has mostly to do with the fact
>>> that now
>>> we can hit the mailbox more often for what is mostly static
>>> information.
>>>
>>> Especially with ethtool we already had to deal with an issue caused
>>> by net-snmp:
>>> https://sourceforge.net/p/e1000/mailman/message/32188362/
>>>
>>> Where net-snmp was being too aggressive when collecting information,
>>> even if most of it was static.
>>
>> Emil, I don't really understand what are u trying to protect here
>> against. If a user would want to shoot him/herself in the leg -
>> he/she would still be able to do it with the other mailbox involving
>> operations like MAC change. So, what's the sense to add useless lines?
>>
>>>
>>>> Perhaps storing the RSS key and the table is better option than
>>>> having to invoke the mailbox on every read.
>>>> I don't think this could work if I understand your proposal correctly.
>>>> The only way to cache the result that would decrease the number of
>>>> mbox
>>>> transactions would be to cache it in the VF. But how could i
>>>> invalidate
>>>> this cache if the table content has been changed by a PF? I think the
>>>> main source of a confusion here is that u assume that PF driver is a
>>>> Linux ixgbe driver that doesn't support an indirection table change at
>>>> the moment. As I have explained above - this should not be assumed.
>>> You keep mentioning other drivers - what other driver do you mean?
>>> All the PF drivers that enable SRIOV are maintained and supported by
>>> Intel.
>>>
>>> For HW older than X550 we can simply not allow the RSS hash to be
>>> modified if the driver is loaded in SRIOV mode.
>>> This way the RSS info can be read once the driver is loaded. For
>>> X550 this can all be done in the VF, so you can avoid calling the
>>> mailbox altogether.
>>> I understand this is a bit limiting, but this is due to HW
>>> limitation anyway (VFs do not have their own RSS config).
>>
>> Let me remind u that Linux, FreeBSD, XEN and DPDK PF drivers are all
>> open source so u can't actually go and "not allow" things. ;) And
>> although Intel developers contribute most of the code there are and
>> will be other contributors too so I doubt the proposed above approach
>> fits the open source spirit well. ;)
>
> Actually these drivers already support multiple OSes just fine. The
> part where I think you are confused is that you assume they all use
> the same Mailbox API which they likely wouldn't. I would suggest
> taking a look at ixgbe_pfvf_api_rev in mbx.h of the VF driver.
> Different OSes have different things that can be supported, so for
> example the ixgbe_mbox_api_20 is reserved for a Solaris based PF/VF
> combination. I would suspect that FreeBSD will likely have to conform
> to the existing APIs, or report that it only supports a different
> version of the mailbox API.
>
>>
>> The user should actually not query the indirection table and a hash
>> key too often. And if he/she does - it should be his/her problem.
>> However, if like with the ixgbevf_set_num_queues() u insist on your
>> way of doing this (on caching the indirection table and hash key) -
>> then please let me know and I will add it. Because, frankly, I care
>> about the PF part of this series much more than for the VF part... ;)
>
> I would say you don't need to cache it, but for 82599 and x540 there
> isn't any need to store more than 3 bits per entry, 384b, or 12 DWORDs
> for the entire RETA of the VF since the hardware can support at most 8
> queues w/ SR-IOV.
Isn't it 2 bits? VF may have up to 8 queues in total but it supports up
to 4 RSS queues (according to MRQC.MRQE and PSRTYPE[n].RQPL description...
> Then you only need one message instead of 3 which will reduce quite a
> bit of the complication with all of this.
>
> Also it might make more sense to start working on displaying this on
> the PF before you start trying to do this on the VF. As far as I know
> ixgbe still doesn't have this functionality and it would make much
> more sense to enable that first on ixgbe before you start trying to
> find a way to feed the data to the VF.
>
> - Alex
next prev parent reply other threads:[~2015-03-26 18:10 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-22 19:01 [PATCH net-next v6 0/7]: ixgbevf: Allow querying VFs RSS indirection table and key Vlad Zolotarov
2015-03-22 19:01 ` [PATCH net-next v6 1/7] if_link: Add an additional parameter to ifla_vf_info for RSS querying Vlad Zolotarov
2015-03-23 10:30 ` Jeff Kirsher
2015-03-22 19:01 ` [PATCH net-next v6 2/7] ixgbe: Add a new netdev op to allow/prevent a VF from querying an RSS info Vlad Zolotarov
2015-03-23 10:30 ` Jeff Kirsher
2015-03-22 19:01 ` [PATCH net-next v6 3/7] ixgbe: Add a RETA query command to VF-PF channel API Vlad Zolotarov
2015-03-23 10:30 ` Jeff Kirsher
2015-03-22 19:01 ` [PATCH net-next v6 4/7] ixgbevf: Add a RETA query code Vlad Zolotarov
2015-03-23 10:31 ` Jeff Kirsher
2015-03-23 23:46 ` Tantilov, Emil S
2015-03-24 10:25 ` Vlad Zolotarov
2015-03-24 18:12 ` Tantilov, Emil S
2015-03-24 19:06 ` Vlad Zolotarov
2015-03-24 21:04 ` Tantilov, Emil S
2015-03-25 9:27 ` Vlad Zolotarov
2015-03-25 18:35 ` Tantilov, Emil S
2015-03-25 20:17 ` Vlad Zolotarov
2015-03-25 21:04 ` Alexander Duyck
2015-03-26 1:09 ` Tantilov, Emil S
2015-03-26 9:35 ` Vlad Zolotarov
2015-03-26 14:40 ` Alexander Duyck
2015-03-26 15:10 ` Vlad Zolotarov
2015-03-26 15:59 ` Alexander Duyck
2015-03-26 16:02 ` Vlad Zolotarov
2015-03-26 16:29 ` Vlad Zolotarov
2015-03-26 18:10 ` Vlad Zolotarov [this message]
2015-03-26 18:25 ` Alexander Duyck
[not found] ` <5511AFD6.2020406@cloudius-systems.com>
2015-03-24 22:50 ` Tantilov, Emil S
2015-03-25 9:29 ` Vlad Zolotarov
2015-03-25 9:39 ` Jeff Kirsher
2015-03-25 9:41 ` Vlad Zolotarov
2015-03-22 19:01 ` [PATCH net-next v6 5/7] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set Vlad Zolotarov
2015-03-23 10:31 ` Jeff Kirsher
2015-03-22 19:01 ` [PATCH net-next v6 6/7] ixgbevf: Add RSS Key query code Vlad Zolotarov
2015-03-23 10:31 ` Jeff Kirsher
2015-03-22 19:01 ` [PATCH net-next v6 7/7] ixgbevf: Add the appropriate ethtool ops to query RSS indirection table and key Vlad Zolotarov
2015-03-23 10:31 ` Jeff Kirsher
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=55144BA5.5040507@cloudius-systems.com \
--to=vladz@cloudius-systems$(echo .)com \
--cc=alexander.h.duyck@redhat$(echo .)com \
--cc=avi@cloudius-systems$(echo .)com \
--cc=donald.c.skidmore@intel$(echo .)com \
--cc=emil.s.tantilov@intel$(echo .)com \
--cc=gleb@cloudius-systems$(echo .)com \
--cc=jeffrey.t.kirsher@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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