[dpdk-dev,v2] bond: inherit maximum rx packet length
Commit Message
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 | 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(-)
Comments
CC'ing follow up conversation which I accidentally took of list.
-------- Forwarded Message --------
Subject: Re: [dpdk-dev] [PATCH] bond: inherit maximum rx packet length
Date: Wed, 4 May 2016 14:11:53 -0400
From: Eric Kinzie <ehkinzie@gmail.com>
To: Declan Doherty <declan.doherty@intel.com>
On Wed May 04 14:53:26 +0100 2016, Declan Doherty wrote:
> On 29/04/16 22:36, Eric Kinzie wrote:
> >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
> >
>
> Hey Eric,
>
> actually I think you're right, we can't change the value dynamically
> once the bonded device has been configured (maybe gating on
> start/stop would be sufficient), but I think we shouldn't explicitly
> gate on the first slave added, as we may be bonding multiple slaves
> each of which could have different max_rx_pktlens. Prior to calling
> dev_configure on the bonded device it should be possible to add any
> slave with any max_rx_pktlen and inherit the minimum value, but once
> the bonded device has been configured we would refuse a slave with a
> max_rx_pktlen value which is smaller than the current value. I think
> this should then satisfy that all slaves meet the minimum
> requirements published by the bonded device?
>
Hi, Declan. I think that would work out ok. I'll write something to
that effect and send another version of the patch.
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
v3 changes:
- allow slaves with any max rx packet length to be added to the bonding
interface until it is configured. After bond_ethdev_configure()
only slaves with max pktlen greater than that of the bond device
can be added. The bond device assumes the smallest of the slave max
pktlen values.
- reset bond device's max_rx_pktlen when last slave removed
Eric Kinzie (1):
bond: inherit maximum rx packet length
drivers/net/bonding/rte_eth_bond_api.c | 18 +++++++++++++++++-
drivers/net/bonding/rte_eth_bond_pmd.c | 6 +++++-
drivers/net/bonding/rte_eth_bond_private.h | 3 +++
3 files changed, 25 insertions(+), 2 deletions(-)
@@ -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 = 2048;
/* Initially allow to choose any offload type */
internals->flow_type_rss_offloads = ETH_RSS_PROTO_MASK;
@@ -331,9 +332,15 @@ __eth_bond_slave_add_lock_free(uint8_t bonded_port_id, uint8_t slave_port_id)
/* Add slave details to bonded device */
slave_eth_dev->data->dev_flags |= RTE_ETH_DEV_BONDED_SLAVE;
- slave_add(internals, slave_eth_dev);
rte_eth_dev_info_get(slave_port_id, &dev_info);
+ if (dev_info.max_rx_pktlen < internals->max_rx_pktlen) {
+ RTE_BOND_LOG(ERR, "Slave (port %u) max_rx_pktlen too small",
+ slave_port_id);
+ return -1;
+ }
+
+ slave_add(internals, slave_eth_dev);
/* We need to store slaves reta_size to be able to synchronize RETA for all
* slave devices even if its sizes are different.
@@ -365,6 +372,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 */
@@ -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;
@@ -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;