[ovs-dev,v2,2/3] netdev-dpdk: avoid reconfiguration on VIRTIO_NET_F_MQ changes

At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
!VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
n and n to 1 continuously, triggering datapath reconfigurations at each
transition.

Limit this by only reconfiguring on increased qp_num.
The previous patch reduced the observed cost of polling disabled queues,
so the only cost is memory.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/netdev-dpdk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

On 16.04.2019 12:45, David Marchand wrote:
> At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
> !VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
> n and n to 1 continuously, triggering datapath reconfigurations at each
> transition.
> 
> Limit this by only reconfiguring on increased qp_num.
> The previous patch reduced the observed cost of polling disabled queues,
> so the only cost is memory.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

It seems weird to ACK my own code. You may credit me in some other way
like From/Suggested/Co-authored tags. Any of them is OK for me.
Please, refer the Documentation/internals/contributing/submitting-patches.rst
for details and additional constraints (like additional sign-offs).
Same for the next patch.

BTW, It's completely unnecessary to send new versions of the patch-set in
reply to the previous. This is specific to DPDK mail-list and not a
common practice. Complicates reading the list and searching for the right
patch versions.

>  lib/netdev-dpdk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 9ba8e67..fc554db 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3510,8 +3510,8 @@ new_device(int vid)
>                  newnode = dev->socket_id;
>              }
>  
> -            if (dev->requested_n_txq != qp_num
> -                || dev->requested_n_rxq != qp_num
> +            if (dev->requested_n_txq < qp_num
> +                || dev->requested_n_rxq < qp_num
>                  || dev->requested_socket_id != newnode) {
>                  dev->requested_socket_id = newnode;
>                  dev->requested_n_rxq = qp_num;
>

On Tue, Apr 16, 2019 at 3:51 PM Ilya Maximets <i.maximets@samsung.com>
wrote:

> On 16.04.2019 12:45, David Marchand wrote:
> > At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
> > !VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
> > n and n to 1 continuously, triggering datapath reconfigurations at each
> > transition.
> >
> > Limit this by only reconfiguring on increased qp_num.
> > The previous patch reduced the observed cost of polling disabled queues,
> > so the only cost is memory.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
>
> It seems weird to ACK my own code. You may credit me in some other way
> like From/Suggested/Co-authored tags. Any of them is OK for me.
> Please, refer the
> Documentation/internals/contributing/submitting-patches.rst
> for details and additional constraints (like additional sign-offs).
> Same for the next patch.
>

I did not want to take all the glory for me :-)
The code is mainly yours, so Co-authored-by + SoB.


> BTW, It's completely unnecessary to send new versions of the patch-set in
> reply to the previous. This is specific to DPDK mail-list and not a
> common practice. Complicates reading the list and searching for the right
> patch versions.
>

Ok, so I won't chain the new version.

On 16/04/2019 10:45, David Marchand wrote:
> At the moment, a malicious guest might negotiate VIRTIO_NET_F_MQ and
> !VIRTIO_NET_F_MQ in a loop which would be seen as qp_num going from 1 to
> n and n to 1 continuously, triggering datapath reconfigurations at each
> transition.
> 
> Limit this by only reconfiguring on increased qp_num.
> The previous patch reduced the observed cost of polling disabled queues,
> so the only cost is memory.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/netdev-dpdk.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 9ba8e67..fc554db 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -3510,8 +3510,8 @@ new_device(int vid)
>                  newnode = dev->socket_id;
>              }
>  
> -            if (dev->requested_n_txq != qp_num
> -                || dev->requested_n_rxq != qp_num
> +            if (dev->requested_n_txq < qp_num
> +                || dev->requested_n_rxq < qp_num
>                  || dev->requested_socket_id != newnode) {
>                  dev->requested_socket_id = newnode;
>                  dev->requested_n_rxq = qp_num;
> 

LGTM
Acked-by: Kevin Traynor <ktraynor@redhat.com>