[1/2] net/bonding: add independent LACP sending function
Checks
Commit Message
From: Long Wu <long.wu@corigine.com>
Sending LACP control packets depends on calling the bonding port's
sending function if we disable dedicated queue. In some cases app
would not call the bonding port's sending function if there are
only LACP control packets and the negotiation between the two
bonding ports will fail.
We add the independent LACP sending function for app. App can call
it by itself and let the negotiation succeed.
Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 58 +++++++++++++++++++++++
drivers/net/bonding/rte_eth_bond_8023ad.h | 19 ++++++++
2 files changed, 77 insertions(+)
Comments
On Thu, 16 Feb 2023 15:15:13 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:
> +void
> +rte_eth_bond_8023ad_lacp_send_one(void *queue)
> +{
> + uint32_t i;
> + uint16_t slave_tx_count;
> + uint16_t active_slave_count;
> + uint16_t active_slave_ids[RTE_MAX_ETHPORTS];
Thinking ahead, all of bonding driver should remove the usage of the
terms master and slave. Perhaps you don't want to introduce new
usages that will have to be fixed.
FYI - there is no usage of master/slave in any of the IEEE standards,
or operating systems other than Linux.
On Thu, Feb 16, 2023 at 11:47:27AM -0800, Stephen Hemminger wrote:
> On Thu, 16 Feb 2023 15:15:13 +0800
> Chaoyong He <chaoyong.he@corigine.com> wrote:
>
> > +void
> > +rte_eth_bond_8023ad_lacp_send_one(void *queue)
> > +{
> > + uint32_t i;
> > + uint16_t slave_tx_count;
> > + uint16_t active_slave_count;
> > + uint16_t active_slave_ids[RTE_MAX_ETHPORTS];
>
> Thinking ahead, all of bonding driver should remove the usage of the
> terms master and slave. Perhaps you don't want to introduce new
> usages that will have to be fixed.
>
> FYI - there is no usage of master/slave in any of the IEEE standards,
> or operating systems other than Linux.
Thanks Stephen,
could we agree on alternative language?
On Mon, 20 Feb 2023 10:46:16 +0100
Simon Horman <simon.horman@corigine.com> wrote:
> On Thu, Feb 16, 2023 at 11:47:27AM -0800, Stephen Hemminger wrote:
> > On Thu, 16 Feb 2023 15:15:13 +0800
> > Chaoyong He <chaoyong.he@corigine.com> wrote:
> >
> > > +void
> > > +rte_eth_bond_8023ad_lacp_send_one(void *queue)
> > > +{
> > > + uint32_t i;
> > > + uint16_t slave_tx_count;
> > > + uint16_t active_slave_count;
> > > + uint16_t active_slave_ids[RTE_MAX_ETHPORTS];
> >
> > Thinking ahead, all of bonding driver should remove the usage of the
> > terms master and slave. Perhaps you don't want to introduce new
> > usages that will have to be fixed.
> >
> > FYI - there is no usage of master/slave in any of the IEEE standards,
> > or operating systems other than Linux.
>
> Thanks Stephen,
>
> could we agree on alternative language?
I did a little looking around and did not come to a great answer.
Looking at FreeBSD (and Solaris) they use lagg for the aggregating device
and laggport for the devices associated with it. Applying same logic
to DPDK would be awkward because it already uses the term "port"
in multiple ways.
Cisco uses the term "port channel group" when configuring link aggregation.
Going that way maybe use channels as the replacement for slave in code like this.
> On Mon, 20 Feb 2023 10:46:16 +0100
> Simon Horman <simon.horman@corigine.com> wrote:
>
> > On Thu, Feb 16, 2023 at 11:47:27AM -0800, Stephen Hemminger wrote:
> > > On Thu, 16 Feb 2023 15:15:13 +0800
> > > Chaoyong He <chaoyong.he@corigine.com> wrote:
> > >
> > > > +void
> > > > +rte_eth_bond_8023ad_lacp_send_one(void *queue) {
> > > > + uint32_t i;
> > > > + uint16_t slave_tx_count;
> > > > + uint16_t active_slave_count;
> > > > + uint16_t active_slave_ids[RTE_MAX_ETHPORTS];
> > >
> > > Thinking ahead, all of bonding driver should remove the usage of the
> > > terms master and slave. Perhaps you don't want to introduce new
> > > usages that will have to be fixed.
> > >
> > > FYI - there is no usage of master/slave in any of the IEEE
> > > standards, or operating systems other than Linux.
> >
> > Thanks Stephen,
> >
> > could we agree on alternative language?
>
> I did a little looking around and did not come to a great answer.
> Looking at FreeBSD (and Solaris) they use lagg for the aggregating device and
> laggport for the devices associated with it. Applying same logic to DPDK
> would be awkward because it already uses the term "port"
> in multiple ways.
>
> Cisco uses the term "port channel group" when configuring link aggregation.
> Going that way maybe use channels as the replacement for slave in code like
> this.
>
How about we use 'main' and 'member' ?
@@ -1757,3 +1757,61 @@ rte_eth_bond_8023ad_dedicated_queues_disable(uint16_t port)
return retval;
}
+
+int
+rte_eth_bond_8023ad_dedicated_queues_get(uint16_t port_id)
+{
+ struct rte_eth_dev *dev;
+ struct bond_dev_private *internals;
+
+ if (valid_bonded_port_id(port_id) != 0)
+ return -EINVAL;
+
+ dev = &rte_eth_devices[port_id];
+ internals = dev->data->dev_private;
+
+ if (internals->mode != BONDING_MODE_8023AD)
+ return -EINVAL;
+
+ return internals->mode4.dedicated_queues.enabled;
+}
+
+void
+rte_eth_bond_8023ad_lacp_send_one(void *queue)
+{
+ uint32_t i;
+ uint16_t slave_tx_count;
+ uint16_t active_slave_count;
+ uint16_t active_slave_ids[RTE_MAX_ETHPORTS];
+ struct bond_tx_queue *bd_tx_q = queue;
+ struct bond_dev_private *internals = bd_tx_q->dev_private;
+
+ active_slave_count = internals->active_slave_count;
+ if (unlikely(active_slave_count == 0))
+ return;
+
+ rte_memcpy(active_slave_ids, internals->active_slaves,
+ sizeof(active_slave_ids[0]) * active_slave_count);
+
+ /* Check for LACP control packets and send if available */
+ for (i = 0; i < active_slave_count; i++) {
+ struct rte_mbuf *ctrl_pkt = NULL;
+ struct port *port = &bond_mode_8023ad_ports[active_slave_ids[i]];
+
+ if (likely(rte_ring_empty(port->tx_ring)))
+ continue;
+
+ if (rte_ring_dequeue(port->tx_ring, (void **)&ctrl_pkt) == 0) {
+ slave_tx_count = rte_eth_tx_prepare(active_slave_ids[i],
+ bd_tx_q->queue_id, &ctrl_pkt, 1);
+ slave_tx_count = rte_eth_tx_burst(active_slave_ids[i],
+ bd_tx_q->queue_id, &ctrl_pkt, slave_tx_count);
+ /*
+ * Re-enqueue LAG control plane packets to buffering
+ * ring if transmission fails so the packet won't lost.
+ */
+ if (slave_tx_count != 1)
+ rte_ring_enqueue(port->tx_ring, ctrl_pkt);
+ }
+ }
+}
@@ -331,4 +331,23 @@ rte_eth_bond_8023ad_agg_selection_get(uint16_t port_id);
int
rte_eth_bond_8023ad_agg_selection_set(uint16_t port_id,
enum rte_bond_8023ad_agg_selection agg_selection);
+
+/**
+ * Get LACP dedicated queues enable/disable for 8023ad
+ * @param port_id Bonding device id
+ * @return
+ * 0 - the port is a bonding mode 4 port with disabled dedicated queue
+ * 1 - the port is a bonding mode 4 port with enabled dedicated queue
+ * -EINVAL - the port is not a bonding port or the bonding port's mode is not 4
+ */
+int
+rte_eth_bond_8023ad_dedicated_queues_get(uint16_t port_id);
+
+/**
+ * Send one LACP packet for bonding port in mode 4 with disabled dedicated queue
+ * @param queue Bonding port's tx queue
+ */
+void
+rte_eth_bond_8023ad_lacp_send_one(void *queue);
+
#endif /* RTE_ETH_BOND_8023AD_H_ */