[dpdk-dev,1/5] bond: use existing enslaved device queues

  If a device to be enslaved already has transmit and/or receive queues
allocated, use those and then create any additional queues that are
necessary.

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 lib/librte_pmd_bond/rte_eth_bond_pmd.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
  

On 2015-04-06 19:01, Eric Kinzie wrote:
> If a device to be enslaved already has transmit and/or receive queues
> allocated, use those and then create any additional queues that are
> necessary.
>
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> ---
>   lib/librte_pmd_bond/rte_eth_bond_pmd.c |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> index c937e6b..4fd7d97 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> +++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> @@ -1318,7 +1318,9 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
>   	}
>
>   	/* Setup Rx Queues */
> -	for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
> +	/* Use existing queues, if any */
> +	for (q_id = slave_eth_dev->data->nb_rx_queues;
> +	     q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
>   		bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id];
>
>   		errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
> @@ -1334,7 +1336,9 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
>   	}
>
>   	/* Setup Tx Queues */
> -	for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
> +	/* Use existing queues, if any */
> +	for (q_id = slave_eth_dev->data->nb_tx_queues;
> +	     q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
>   		bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id];
>
>   		errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
>

Why you want to do that?

As far as I am aware (but Declan Doherty should speak here to) purpose
of this part of code is to have configuration of queues in slaves
consistent with bd_rx_q/bd_tx_q. If you skip reconfiguration of queues
that are already configured in port you can have them configured
in different way after enslaving.

So again: what is the purpose of doing so?
  

On Fri Apr 10 09:40:09 +0200 2015, Pawel Wodkowski wrote:
> On 2015-04-06 19:01, Eric Kinzie wrote:
> >If a device to be enslaved already has transmit and/or receive queues
> >allocated, use those and then create any additional queues that are
> >necessary.
> >
> >Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> >---
> >  lib/librte_pmd_bond/rte_eth_bond_pmd.c |    8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> >index c937e6b..4fd7d97 100644
> >--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> >+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
> >@@ -1318,7 +1318,9 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> >  	}
> >
> >  	/* Setup Rx Queues */
> >-	for (q_id = 0; q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
> >+	/* Use existing queues, if any */
> >+	for (q_id = slave_eth_dev->data->nb_rx_queues;
> >+	     q_id < bonded_eth_dev->data->nb_rx_queues; q_id++) {
> >  		bd_rx_q = (struct bond_rx_queue *)bonded_eth_dev->data->rx_queues[q_id];
> >
> >  		errval = rte_eth_rx_queue_setup(slave_eth_dev->data->port_id, q_id,
> >@@ -1334,7 +1336,9 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> >  	}
> >
> >  	/* Setup Tx Queues */
> >-	for (q_id = 0; q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
> >+	/* Use existing queues, if any */
> >+	for (q_id = slave_eth_dev->data->nb_tx_queues;
> >+	     q_id < bonded_eth_dev->data->nb_tx_queues; q_id++) {
> >  		bd_tx_q = (struct bond_tx_queue *)bonded_eth_dev->data->tx_queues[q_id];
> >
> >  		errval = rte_eth_tx_queue_setup(slave_eth_dev->data->port_id, q_id,
> >
> 
> Why you want to do that?
> 
> As far as I am aware (but Declan Doherty should speak here to) purpose
> of this part of code is to have configuration of queues in slaves
> consistent with bd_rx_q/bd_tx_q. If you skip reconfiguration of queues
> that are already configured in port you can have them configured
> in different way after enslaving.
> 
> So again: what is the purpose of doing so?
> 
> -- 
> Pawel

Pawel,

I generally test things I've just built using virtio devices and calling
rte_eth_tx_queue_setup() more than once for a given queue id fails.
However, it seems that most PMDs allow re-allocating device queues while
virtio does not (xenvirt also seems to lack this functionality), so I
don't think my approach here is right.  I'll remove this patch when I
send the next version of this series.

Thanks,

Eric
  

> 
> Pawel,
> 
> I generally test things I've just built using virtio devices and calling
> rte_eth_tx_queue_setup() more than once for a given queue id fails.
> However, it seems that most PMDs allow re-allocating device queues while
> virtio does not (xenvirt also seems to lack this functionality), so I
> don't think my approach here is right.  I'll remove this patch when I
> send the next version of this series.
> 
> Thanks,
> 
> Eric

Maybe you should rise this as separate issue maintainers of these drivers?