[dpdk-dev] bond: inherit maximum rx packet length

Message ID 1460654624-2066-1-git-send-email-ehkinzie@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Eric Kinzie April 14, 2016, 5:23 p.m. UTC
  Instead of a hard-coded maximum receive length, allow the bond interface
  to inherit this limit from the first slave added.  This allows
  an application that uses jumbo frames to pass realistic values to
  rte_eth_dev_configure without causing an error.

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/bonding/rte_eth_bond_api.c     |    4 ++++
 drivers/net/bonding/rte_eth_bond_pmd.c     |    2 +-
 drivers/net/bonding/rte_eth_bond_private.h |    2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)
  

Comments

Doherty, Declan April 26, 2016, 10:51 a.m. UTC | #1
On 14/04/16 18:23, Eric Kinzie wrote:
>    Instead of a hard-coded maximum receive length, allow the bond interface
>    to inherit this limit from the first slave added.  This allows
>    an application that uses jumbo frames to pass realistic values to
>    rte_eth_dev_configure without causing an error.
>
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> ---
...
>

Hey Eric, just one small thing, I think it probably makes sense to 
return the max rx pktlen for all slaves, so as we add each slave just 
check if that the slave being value is larger than the current value.

@@ -385,6 +389,10 @@ __eth_bond_slave_add_lock_free(uint8_t 
bonded_port_id, uint8_t slave_port_id)
                 internals->tx_offload_capa &= dev_info.tx_offload_capa;
                 internals->flow_type_rss_offloads &= 
dev_info.flow_type_rss_offloads;

+               /* If new slave's max rx packet size is larger than 
current value then override */
+               if (dev_info.max_rx_pktlen > internals->max_rx_pktlen)
+                       internals->max_rx_pktlen = dev_info.max_rx_pktlen;
+

Declan
  
Eric Kinzie April 29, 2016, 9:36 p.m. UTC | #2
On Tue Apr 26 11:51:53 +0100 2016, Declan Doherty wrote:
> On 14/04/16 18:23, Eric Kinzie wrote:
> >   Instead of a hard-coded maximum receive length, allow the bond interface
> >   to inherit this limit from the first slave added.  This allows
> >   an application that uses jumbo frames to pass realistic values to
> >   rte_eth_dev_configure without causing an error.
> >
> >Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> >---
> ...
> >
> 
> Hey Eric, just one small thing, I think it probably makes sense to
> return the max rx pktlen for all slaves, so as we add each slave
> just check if that the slave being value is larger than the current
> value.
> 
> @@ -385,6 +389,10 @@ __eth_bond_slave_add_lock_free(uint8_t
> bonded_port_id, uint8_t slave_port_id)
>                 internals->tx_offload_capa &= dev_info.tx_offload_capa;
>                 internals->flow_type_rss_offloads &=
> dev_info.flow_type_rss_offloads;
> 
> +               /* If new slave's max rx packet size is larger than
> current value then override */
> +               if (dev_info.max_rx_pktlen > internals->max_rx_pktlen)
> +                       internals->max_rx_pktlen = dev_info.max_rx_pktlen;
> +
> 
> Declan

Declan, I sent an updated patch but now release that I mis-read your
comments.  Is it a good idea to change the value once it's been set?
My patch now refuses to add a slave with a pktlen value that's smaller
than that of the first slave.

Eric
  
Eric Kinzie April 29, 2016, 10:30 p.m. UTC | #3
v2 changes:
 - remove type cast on constant
 - check max_rx_pktlen when adding a slave to make sure it is >= max
   packet length of existing slave interfaces

Eric Kinzie (1):
  bond: inherit maximum rx packet length

 drivers/net/bonding/rte_eth_bond_api.c     | 12 +++++++++++-
 drivers/net/bonding/rte_eth_bond_pmd.c     |  2 +-
 drivers/net/bonding/rte_eth_bond_private.h |  2 ++
 3 files changed, 14 insertions(+), 2 deletions(-)
  

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index e9247b5..b763b37 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -247,6 +247,7 @@  rte_eth_bond_create(const char *name, uint8_t mode, uint8_t socket_id)
 	internals->active_slave_count = 0;
 	internals->rx_offload_capa = 0;
 	internals->tx_offload_capa = 0;
+	internals->max_rx_pktlen = (uint32_t)2048;
 
 	/* Initially allow to choose any offload type */
 	internals->flow_type_rss_offloads = ETH_RSS_PROTO_MASK;
@@ -365,6 +366,9 @@  __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
 		internals->tx_offload_capa = dev_info.tx_offload_capa;
 		internals->flow_type_rss_offloads = dev_info.flow_type_rss_offloads;
 
+		/* Inherit first slave's max rx packet size */
+		internals->max_rx_pktlen = dev_info.max_rx_pktlen;
+
 	} else {
 		/* Check slave link properties are supported if props are set,
 		 * all slaves must be the same */
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 54788cf..189fb47 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1650,7 +1650,7 @@  bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	dev_info->max_mac_addrs = 1;
 
-	dev_info->max_rx_pktlen = (uint32_t)2048;
+	dev_info->max_rx_pktlen = internals->max_rx_pktlen;
 
 	dev_info->max_rx_queues = (uint16_t)128;
 	dev_info->max_tx_queues = (uint16_t)512;
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index 8312397..79ca69d 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -169,6 +169,8 @@  struct bond_dev_private {
 
 	struct rte_kvargs *kvlist;
 	uint8_t slave_update_idx;
+
+	uint32_t max_rx_pktlen;
 };
 
 extern const struct eth_dev_ops default_dev_ops;