[dpdk-dev,4/5] bond mode 4: allow external state machine

Message ID 1428339685-27686-5-git-send-email-ehkinzie@gmail.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Eric Kinzie April 6, 2015, 5:01 p.m. UTC
  Provide functions to allow an external 802.3ad state machine to transmit
  and recieve LACPDUs and to set the collection/distribution flags on
  slave interfaces.

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 lib/librte_pmd_bond/rte_eth_bond_8023ad.c         |  175 +++++++++++++++++++++
 lib/librte_pmd_bond/rte_eth_bond_8023ad.h         |   44 ++++++
 lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h |    2 +
 3 files changed, 221 insertions(+)
  

Comments

Wodkowski, PawelX April 7, 2015, 2:18 p.m. UTC | #1
On 2015-04-06 19:01, Eric Kinzie wrote:

Interesting patch. I will closer look at this tomorrow.

For now I have first comments:

> +static void bond_mode_8023ad_ext_periodic_cb(void *arg);
> +
>   #ifdef RTE_LIBRTE_BOND_DEBUG_8023AD
>   #define MODE4_DEBUG(fmt, ...) RTE_LOG(DEBUG, PMD, "%6u [Port %u: %s] " fmt, \
>   			bond_dbg_get_time_diff_ms(), slave_id, \
> @@ -1014,6 +1016,8 @@ bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
>   	conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
>   	conf->update_timeout_ms = mode4->update_timeout_us / 1000;
>   	conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
> +	conf->slowrx_cb = mode4->slowrx_cb;
> +	conf->external_sm = mode4->external_sm;

mode4->external_sm flag realy needed? Why do not use mode4->slowrx_cb as 
external state machine indicator?

>   }
>
>   void
> @@ -1035,6 +1039,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
>   		conf->tx_period_ms = BOND_8023AD_TX_MACHINE_PERIOD_MS;
>   		conf->rx_marker_period_ms = BOND_8023AD_RX_MARKER_PERIOD_MS;
>   		conf->update_timeout_ms = BOND_MODE_8023AX_UPDATE_TIMEOUT_MS;
> +		conf->slowrx_cb = NULL;
> +		conf->external_sm = 0;
>   	}
>
>   	mode4->fast_periodic_timeout = conf->fast_periodic_ms * ms_ticks;
> @@ -1045,6 +1051,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
>   	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
>   	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
>   	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
> +	mode4->slowrx_cb = conf->slowrx_cb;
> +	mode4->external_sm = conf->external_sm;
>   }
>
>   int
> @@ -1062,6 +1070,13 @@ bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
>   int
>   bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
>   {
> +	struct bond_dev_private *internals = bond_dev->data->dev_private;
> +	struct mode8023ad_private *mode4 = &internals->mode4;
> +
> +	if (mode4->external_sm)
> +		return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
> +			&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> +
>   	return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
>   			&bond_mode_8023ad_periodic_cb, bond_dev);
>   }
> @@ -1069,6 +1084,13 @@ bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
>   void
>   bond_mode_8023ad_stop(struct rte_eth_dev *bond_dev)
>   {
> +	struct bond_dev_private *internals = bond_dev->data->dev_private;
> +	struct mode8023ad_private *mode4 = &internals->mode4;
> +
> +	if (mode4->external_sm) {

This is bad idea. If bond_mode_8023ad_setup will be called you might 
have two handlers running for while. You should stop mode 4 by invoking 
bond_mode_8023ad_stop() before you set mode4->external_sm and then, if 
mode 4 was running, start it again.

Also, maybe a renaming "external_sm" to "state_machine_cb", set it to 
against default one and using it without "if()" will simplify code. It 
is no crucial but will eliminate couple of if's. In 
rte_eth_bond_8023ad_ext_slowtx() you can compare it against default one.

> +		rte_eal_alarm_cancel(&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> +		return;
> +	}
>   	rte_eal_alarm_cancel(&bond_mode_8023ad_periodic_cb, bond_dev);
>   }
>
> @@ -1215,3 +1237,156 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
>   	info->agg_port_id = port->aggregator_port_id;
>   	return 0;
>   }
  
Wodkowski, PawelX April 7, 2015, 2:37 p.m. UTC | #2
On 2015-04-07 16:18, Pawel Wodkowski wrote:

>
> Also, maybe a renaming "external_sm" to "state_machine_cb", set it to
> against default one and using it without "if()" will simplify code. It
> is no crucial but will eliminate couple of if's. In
> rte_eth_bond_8023ad_ext_slowtx() you can compare it against default one.

Oh, I read what I wrote :)
Please ignore that.
  
Eric Kinzie April 7, 2015, 5:31 p.m. UTC | #3
On Tue Apr 07 16:18:08 +0200 2015, Pawel Wodkowski wrote:
> On 2015-04-06 19:01, Eric Kinzie wrote:
> 
> Interesting patch. I will closer look at this tomorrow.
> 
> For now I have first comments:
> 
> >+static void bond_mode_8023ad_ext_periodic_cb(void *arg);
> >+
> >  #ifdef RTE_LIBRTE_BOND_DEBUG_8023AD
> >  #define MODE4_DEBUG(fmt, ...) RTE_LOG(DEBUG, PMD, "%6u [Port %u: %s] " fmt, \
> >  			bond_dbg_get_time_diff_ms(), slave_id, \
> >@@ -1014,6 +1016,8 @@ bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
> >  	conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
> >  	conf->update_timeout_ms = mode4->update_timeout_us / 1000;
> >  	conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
> >+	conf->slowrx_cb = mode4->slowrx_cb;
> >+	conf->external_sm = mode4->external_sm;
> 
> mode4->external_sm flag realy needed? Why do not use
> mode4->slowrx_cb as external state machine indicator?

I'll remove the external_sm flag.  You're correct, it's not needed.


> >  }
> >
> >  void
> >@@ -1035,6 +1039,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
> >  		conf->tx_period_ms = BOND_8023AD_TX_MACHINE_PERIOD_MS;
> >  		conf->rx_marker_period_ms = BOND_8023AD_RX_MARKER_PERIOD_MS;
> >  		conf->update_timeout_ms = BOND_MODE_8023AX_UPDATE_TIMEOUT_MS;
> >+		conf->slowrx_cb = NULL;
> >+		conf->external_sm = 0;
> >  	}
> >
> >  	mode4->fast_periodic_timeout = conf->fast_periodic_ms * ms_ticks;
> >@@ -1045,6 +1051,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
> >  	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
> >  	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
> >  	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
> >+	mode4->slowrx_cb = conf->slowrx_cb;
> >+	mode4->external_sm = conf->external_sm;
> >  }
> >
> >  int
> >@@ -1062,6 +1070,13 @@ bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
> >  int
> >  bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
> >  {
> >+	struct bond_dev_private *internals = bond_dev->data->dev_private;
> >+	struct mode8023ad_private *mode4 = &internals->mode4;
> >+
> >+	if (mode4->external_sm)
> >+		return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
> >+			&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> >+
> >  	return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
> >  			&bond_mode_8023ad_periodic_cb, bond_dev);
> >  }
> >@@ -1069,6 +1084,13 @@ bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
> >  void
> >  bond_mode_8023ad_stop(struct rte_eth_dev *bond_dev)
> >  {
> >+	struct bond_dev_private *internals = bond_dev->data->dev_private;
> >+	struct mode8023ad_private *mode4 = &internals->mode4;
> >+
> >+	if (mode4->external_sm) {
> 
> This is bad idea. If bond_mode_8023ad_setup will be called you might
> have two handlers running for while. You should stop mode 4 by
> invoking bond_mode_8023ad_stop() before you set mode4->external_sm
> and then, if mode 4 was running, start it again.

Thanks, I'll make that change.



> Also, maybe a renaming "external_sm" to "state_machine_cb", set it
> to against default one and using it without "if()" will simplify
> code. It is no crucial but will eliminate couple of if's. In
> rte_eth_bond_8023ad_ext_slowtx() you can compare it against default
> one.
> 
> >+		rte_eal_alarm_cancel(&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> >+		return;
> >+	}
> >  	rte_eal_alarm_cancel(&bond_mode_8023ad_periodic_cb, bond_dev);
> >  }
> >
> >@@ -1215,3 +1237,156 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
> >  	info->agg_port_id = port->aggregator_port_id;
> >  	return 0;
> >  }
> 
> -- 
> Pawel
  
Wodkowski, PawelX April 10, 2015, 8:29 a.m. UTC | #4
Hi Eric
Please see my comments.

On 2015-04-06 19:01, Eric Kinzie wrote:
>    Provide functions to allow an external 802.3ad state machine to transmit
>    and recieve LACPDUs and to set the collection/distribution flags on
>    slave interfaces.
>
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> ---
>   lib/librte_pmd_bond/rte_eth_bond_8023ad.c         |  175 +++++++++++++++++++++
>   lib/librte_pmd_bond/rte_eth_bond_8023ad.h         |   44 ++++++
>   lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h |    2 +
>   3 files changed, 221 insertions(+)
>
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad.c b/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
> index 1009d5b..29cd962 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
> +++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
> @@ -42,6 +42,8 @@
>
>   #include "rte_eth_bond_private.h"
>
> +static void bond_mode_8023ad_ext_periodic_cb(void *arg);
> +
>   #ifdef RTE_LIBRTE_BOND_DEBUG_8023AD
>   #define MODE4_DEBUG(fmt, ...) RTE_LOG(DEBUG, PMD, "%6u [Port %u: %s] " fmt, \
>   			bond_dbg_get_time_diff_ms(), slave_id, \
> @@ -1014,6 +1016,8 @@ bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
>   	conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
>   	conf->update_timeout_ms = mode4->update_timeout_us / 1000;
>   	conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
> +	conf->slowrx_cb = mode4->slowrx_cb;
> +	conf->external_sm = mode4->external_sm;
>   }
>
>   void
> @@ -1035,6 +1039,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
>   		conf->tx_period_ms = BOND_8023AD_TX_MACHINE_PERIOD_MS;
>   		conf->rx_marker_period_ms = BOND_8023AD_RX_MARKER_PERIOD_MS;
>   		conf->update_timeout_ms = BOND_MODE_8023AX_UPDATE_TIMEOUT_MS;
> +		conf->slowrx_cb = NULL;
> +		conf->external_sm = 0;
>   	}
>
>   	mode4->fast_periodic_timeout = conf->fast_periodic_ms * ms_ticks;
> @@ -1045,6 +1051,8 @@ bond_mode_8023ad_setup(struct rte_eth_dev *dev,
>   	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
>   	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
>   	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
> +	mode4->slowrx_cb = conf->slowrx_cb;
> +	mode4->external_sm = conf->external_sm;
>   }
>
>   int
> @@ -1062,6 +1070,13 @@ bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
>   int
>   bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
>   {
> +	struct bond_dev_private *internals = bond_dev->data->dev_private;
> +	struct mode8023ad_private *mode4 = &internals->mode4;
> +
> +	if (mode4->external_sm)
> +		return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
> +			&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> +
>   	return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
>   			&bond_mode_8023ad_periodic_cb, bond_dev);
>   }
> @@ -1069,6 +1084,13 @@ bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
>   void
>   bond_mode_8023ad_stop(struct rte_eth_dev *bond_dev)
>   {
> +	struct bond_dev_private *internals = bond_dev->data->dev_private;
> +	struct mode8023ad_private *mode4 = &internals->mode4;
> +
> +	if (mode4->external_sm) {
> +		rte_eal_alarm_cancel(&bond_mode_8023ad_ext_periodic_cb, bond_dev);
> +		return;
> +	}
>   	rte_eal_alarm_cancel(&bond_mode_8023ad_periodic_cb, bond_dev);
>   }
>
> @@ -1215,3 +1237,156 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
>   	info->agg_port_id = port->aggregator_port_id;
>   	return 0;
>   }
> +
> +int
> +rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled)
> +{
> +	struct rte_eth_dev *bond_dev;
> +	struct bond_dev_private *internals;
> +	struct mode8023ad_private *mode4;
> +	struct port *port;
> +
> +	if (valid_bonded_port_id(port_id) != 0 ||
> +			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)

The rte_eth_bond_mode_get() function already check if given port_id is 
valid bonded device so you can remove valid_bonded_port_id() here.

You should check here is port is started.

> +		return -EINVAL;
> +
> +	bond_dev = &rte_eth_devices[port_id];
> +
> +	internals = bond_dev->data->dev_private;
> +	if (find_slave_by_id(internals->active_slaves,
> +			internals->active_slave_count, slave_id) ==
> +				internals->active_slave_count)
> +		return -EINVAL;
> +
> +	mode4 = &internals->mode4;
> +	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
> +		return -EINVAL;
> +
> +	port = &mode_8023ad_ports[slave_id];
> +
> +	if (enabled)
> +		ACTOR_STATE_SET(port, COLLECTING);
> +	else
> +		ACTOR_STATE_CLR(port, COLLECTING);
> +
> +	return 0;
> +}
> +
> +int
> +rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled)
> +{
> +	struct rte_eth_dev *bond_dev;
> +	struct bond_dev_private *internals;
> +	struct mode8023ad_private *mode4;
> +	struct port *port;
> +
> +	if (valid_bonded_port_id(port_id) != 0 ||
> +			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)

The same as above.
You can remove call to valid_bonded_port_id().
You should check here if port is started.

> +		return -EINVAL;
> +
> +	bond_dev = &rte_eth_devices[port_id];
> +
> +	internals = bond_dev->data->dev_private;
> +	if (find_slave_by_id(internals->active_slaves,
> +			internals->active_slave_count, slave_id) ==
> +				internals->active_slave_count)
> +		return -EINVAL;
> +
> +	mode4 = &internals->mode4;
> +	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
> +		return -EINVAL;
> +
> +	port = &mode_8023ad_ports[slave_id];
> +
> +	if (enabled)
> +		ACTOR_STATE_SET(port, DISTRIBUTING);
> +	else
> +		ACTOR_STATE_CLR(port, DISTRIBUTING);
> +
> +	return 0;
> +}
> +
> +int
> +rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
> +		struct rte_mbuf *lacp_pkt)
> +{
> +	struct rte_eth_dev *bond_dev;
> +	struct bond_dev_private *internals;
> +	struct mode8023ad_private *mode4;
> +	struct port *port;
> +
> +	if (valid_bonded_port_id(port_id) != 0 ||
> +			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)

The same as above.
You can remove call to valid_bonded_port_id().
You should check here if port is started.

> +		return -EINVAL;
> +
> +	bond_dev = &rte_eth_devices[port_id];
> +
> +	internals = bond_dev->data->dev_private;
> +	if (find_slave_by_id(internals->active_slaves,
> +			internals->active_slave_count, slave_id) ==
> +				internals->active_slave_count)
> +		return -EINVAL;
> +
> +	mode4 = &internals->mode4;
> +	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
> +		return -EINVAL;
> +
> +	port = &mode_8023ad_ports[slave_id];
> +
> +	if (port->tx_ring == NULL)
> +		return -EINVAL;

If bonded port is started and is in mode 4 the tx_ring might not be
NULL. If it is NULL then it is a bug, and should not be hidden by
returning error code.

> +
> +	if (rte_pktmbuf_pkt_len(lacp_pkt) < sizeof(struct lacpdu_header))
> +		return -EINVAL;
> +
> +	struct lacpdu_header *lacp;
> +
> +	/* only enqueue LACPDUs */
> +	lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
> +	if (lacp->lacpdu.subtype != SLOW_SUBTYPE_LACP) {
> +		rte_pktmbuf_free(lacp_pkt);
> +		return 0;
> +	}

I would rather return here an error and do not free the packet.

> +
> +	if (rte_ring_enqueue(port->tx_ring, lacp_pkt) == -ENOBUFS)
> +		return -ENOBUFS;
> +
> +	MODE4_DEBUG("sending LACP frame\n");
> +	return 0;
> +}
> +
> +static void
> +bond_mode_8023ad_ext_periodic_cb(void *arg)
> +{
> +	struct rte_eth_dev *bond_dev = arg;
> +	struct bond_dev_private *internals = bond_dev->data->dev_private;
> +	struct mode8023ad_private *mode4 = &internals->mode4;
> +	struct port *port;
> +	void *pkt = NULL;
> +	uint16_t i, slave_id;
> +
> +	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
> +		return;

Please remove this check. It might not happen at runtime.
If you like you can place here RTE_VERIFY().

> +
> +	for (i = 0; i < internals->active_slave_count; i++) {
> +		slave_id = internals->active_slaves[i];
> +		port = &mode_8023ad_ports[slave_id];
> +
> +		if (rte_ring_dequeue(port->rx_ring, &pkt) == 0) {
> +			struct rte_mbuf *lacp_pkt = pkt;
> +			struct lacpdu_header *lacp;
> +
> +			lacp = rte_pktmbuf_mtod(lacp_pkt,
> +						struct lacpdu_header *);
> +			RTE_VERIFY(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
> +
> +			/* This is LACP frame so pass it to rx callback.
> +			 * Callback is responsible for freeing mbuf.
> +			 */
> +			mode4->slowrx_cb(slave_id, lacp_pkt);
> +		}
> +	}
> +
> +	rte_eal_alarm_set(internals->mode4.update_timeout_us,
> +			bond_mode_8023ad_ext_periodic_cb, arg);
> +}
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad.h b/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
> index ebd0e93..c196584 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
> +++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
> @@ -64,6 +64,8 @@ extern "C" {
>   #define MARKER_TLV_TYPE_INFO                0x01
>   #define MARKER_TLV_TYPE_RESP                0x02
>
> +typedef void (*rte_eth_bond_8023ad_ext_slowrx_fn)(uint8_t slave_id, struct rte_mbuf *lacp_pkt);
> +
>   enum rte_bond_8023ad_selection {
>   	UNSELECTED,
>   	STANDBY,
> @@ -157,6 +159,8 @@ struct rte_eth_bond_8023ad_conf {
>   	uint32_t tx_period_ms;
>   	uint32_t rx_marker_period_ms;
>   	uint32_t update_timeout_ms;
> +	rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
> +	uint8_t external_sm;
>   };
>
>   struct rte_eth_bond_8023ad_slave_info {
> @@ -219,4 +223,44 @@ rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
>   }
>   #endif
>
> +/**
> + * Configure a slave port to start collecting.
> + *
> + * @param port_id	Bonding device id
> + * @param slave_id	Port id of valid slave.
> + * @param enabled	Non-zero when collection enabled.
> + * @return
> + *   0 - if ok
> + *   -EINVAL if slave is not valid.
> + */
> +int
> +rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled);
> +
> +/**
> + * Configure a slave port to start distributing.
> + *
> + * @param port_id	Bonding device id
> + * @param slave_id	Port id of valid slave.
> + * @param enabled	Non-zero when distribution enabled.
> + * @return
> + *   0 - if ok
> + *   -EINVAL if slave is not valid.
> + */
> +int
> +rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled);
> +
> +/**
> + * LACPDU transmit path for external 802.3ad state machine
> + *
> + * @param port_id	Bonding device id
> + * @param slave_id	Port ID of valid slave device.
> + * @param lacp_pkt	mbuf containing LACPDU.
> + *
> + * @return
> + *   0 on success, negative value otherwise.

Please include informations when packet is given back to the caller and 
when its ownership is taken by bonding.

> + */
> +int
> +rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
> +		struct rte_mbuf *lacp_pkt);
> +
>   #endif /* RTE_ETH_BOND_8023AD_H_ */
> diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h b/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
> index 8adee70..9e15ece 100644
> --- a/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
> +++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
> @@ -173,6 +173,8 @@ struct mode8023ad_private {
>   	uint64_t tx_period_timeout;
>   	uint64_t rx_marker_timeout;
>   	uint64_t update_timeout_us;
> +	rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
> +	uint8_t external_sm;
>   };
>
>   /**
>
  

Patch

diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad.c b/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
index 1009d5b..29cd962 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad.c
@@ -42,6 +42,8 @@ 
 
 #include "rte_eth_bond_private.h"
 
+static void bond_mode_8023ad_ext_periodic_cb(void *arg);
+
 #ifdef RTE_LIBRTE_BOND_DEBUG_8023AD
 #define MODE4_DEBUG(fmt, ...) RTE_LOG(DEBUG, PMD, "%6u [Port %u: %s] " fmt, \
 			bond_dbg_get_time_diff_ms(), slave_id, \
@@ -1014,6 +1016,8 @@  bond_mode_8023ad_conf_get(struct rte_eth_dev *dev,
 	conf->tx_period_ms = mode4->tx_period_timeout / ms_ticks;
 	conf->update_timeout_ms = mode4->update_timeout_us / 1000;
 	conf->rx_marker_period_ms = mode4->rx_marker_timeout / ms_ticks;
+	conf->slowrx_cb = mode4->slowrx_cb;
+	conf->external_sm = mode4->external_sm;
 }
 
 void
@@ -1035,6 +1039,8 @@  bond_mode_8023ad_setup(struct rte_eth_dev *dev,
 		conf->tx_period_ms = BOND_8023AD_TX_MACHINE_PERIOD_MS;
 		conf->rx_marker_period_ms = BOND_8023AD_RX_MARKER_PERIOD_MS;
 		conf->update_timeout_ms = BOND_MODE_8023AX_UPDATE_TIMEOUT_MS;
+		conf->slowrx_cb = NULL;
+		conf->external_sm = 0;
 	}
 
 	mode4->fast_periodic_timeout = conf->fast_periodic_ms * ms_ticks;
@@ -1045,6 +1051,8 @@  bond_mode_8023ad_setup(struct rte_eth_dev *dev,
 	mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
 	mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
 	mode4->update_timeout_us = conf->update_timeout_ms * 1000;
+	mode4->slowrx_cb = conf->slowrx_cb;
+	mode4->external_sm = conf->external_sm;
 }
 
 int
@@ -1062,6 +1070,13 @@  bond_mode_8023ad_enable(struct rte_eth_dev *bond_dev)
 int
 bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
 {
+	struct bond_dev_private *internals = bond_dev->data->dev_private;
+	struct mode8023ad_private *mode4 = &internals->mode4;
+
+	if (mode4->external_sm)
+		return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
+			&bond_mode_8023ad_ext_periodic_cb, bond_dev);
+
 	return rte_eal_alarm_set(BOND_MODE_8023AX_UPDATE_TIMEOUT_MS * 1000,
 			&bond_mode_8023ad_periodic_cb, bond_dev);
 }
@@ -1069,6 +1084,13 @@  bond_mode_8023ad_start(struct rte_eth_dev *bond_dev)
 void
 bond_mode_8023ad_stop(struct rte_eth_dev *bond_dev)
 {
+	struct bond_dev_private *internals = bond_dev->data->dev_private;
+	struct mode8023ad_private *mode4 = &internals->mode4;
+
+	if (mode4->external_sm) {
+		rte_eal_alarm_cancel(&bond_mode_8023ad_ext_periodic_cb, bond_dev);
+		return;
+	}
 	rte_eal_alarm_cancel(&bond_mode_8023ad_periodic_cb, bond_dev);
 }
 
@@ -1215,3 +1237,156 @@  rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
 	info->agg_port_id = port->aggregator_port_id;
 	return 0;
 }
+
+int
+rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled)
+{
+	struct rte_eth_dev *bond_dev;
+	struct bond_dev_private *internals;
+	struct mode8023ad_private *mode4;
+	struct port *port;
+
+	if (valid_bonded_port_id(port_id) != 0 ||
+			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+		return -EINVAL;
+
+	bond_dev = &rte_eth_devices[port_id];
+
+	internals = bond_dev->data->dev_private;
+	if (find_slave_by_id(internals->active_slaves,
+			internals->active_slave_count, slave_id) ==
+				internals->active_slave_count)
+		return -EINVAL;
+
+	mode4 = &internals->mode4;
+	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
+		return -EINVAL;
+
+	port = &mode_8023ad_ports[slave_id];
+
+	if (enabled)
+		ACTOR_STATE_SET(port, COLLECTING);
+	else
+		ACTOR_STATE_CLR(port, COLLECTING);
+
+	return 0;
+}
+
+int
+rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled)
+{
+	struct rte_eth_dev *bond_dev;
+	struct bond_dev_private *internals;
+	struct mode8023ad_private *mode4;
+	struct port *port;
+
+	if (valid_bonded_port_id(port_id) != 0 ||
+			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+		return -EINVAL;
+
+	bond_dev = &rte_eth_devices[port_id];
+
+	internals = bond_dev->data->dev_private;
+	if (find_slave_by_id(internals->active_slaves,
+			internals->active_slave_count, slave_id) ==
+				internals->active_slave_count)
+		return -EINVAL;
+
+	mode4 = &internals->mode4;
+	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
+		return -EINVAL;
+
+	port = &mode_8023ad_ports[slave_id];
+
+	if (enabled)
+		ACTOR_STATE_SET(port, DISTRIBUTING);
+	else
+		ACTOR_STATE_CLR(port, DISTRIBUTING);
+
+	return 0;
+}
+
+int
+rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
+		struct rte_mbuf *lacp_pkt)
+{
+	struct rte_eth_dev *bond_dev;
+	struct bond_dev_private *internals;
+	struct mode8023ad_private *mode4;
+	struct port *port;
+
+	if (valid_bonded_port_id(port_id) != 0 ||
+			rte_eth_bond_mode_get(port_id) != BONDING_MODE_8023AD)
+		return -EINVAL;
+
+	bond_dev = &rte_eth_devices[port_id];
+
+	internals = bond_dev->data->dev_private;
+	if (find_slave_by_id(internals->active_slaves,
+			internals->active_slave_count, slave_id) ==
+				internals->active_slave_count)
+		return -EINVAL;
+
+	mode4 = &internals->mode4;
+	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
+		return -EINVAL;
+
+	port = &mode_8023ad_ports[slave_id];
+
+	if (port->tx_ring == NULL)
+		return -EINVAL;
+
+	if (rte_pktmbuf_pkt_len(lacp_pkt) < sizeof(struct lacpdu_header))
+		return -EINVAL;
+
+	struct lacpdu_header *lacp;
+
+	/* only enqueue LACPDUs */
+	lacp = rte_pktmbuf_mtod(lacp_pkt, struct lacpdu_header *);
+	if (lacp->lacpdu.subtype != SLOW_SUBTYPE_LACP) {
+		rte_pktmbuf_free(lacp_pkt);
+		return 0;
+	}
+
+	if (rte_ring_enqueue(port->tx_ring, lacp_pkt) == -ENOBUFS)
+		return -ENOBUFS;
+
+	MODE4_DEBUG("sending LACP frame\n");
+	return 0;
+}
+
+static void
+bond_mode_8023ad_ext_periodic_cb(void *arg)
+{
+	struct rte_eth_dev *bond_dev = arg;
+	struct bond_dev_private *internals = bond_dev->data->dev_private;
+	struct mode8023ad_private *mode4 = &internals->mode4;
+	struct port *port;
+	void *pkt = NULL;
+	uint16_t i, slave_id;
+
+	if (mode4->slowrx_cb == NULL || !mode4->external_sm)
+		return;
+
+	for (i = 0; i < internals->active_slave_count; i++) {
+		slave_id = internals->active_slaves[i];
+		port = &mode_8023ad_ports[slave_id];
+
+		if (rte_ring_dequeue(port->rx_ring, &pkt) == 0) {
+			struct rte_mbuf *lacp_pkt = pkt;
+			struct lacpdu_header *lacp;
+
+			lacp = rte_pktmbuf_mtod(lacp_pkt,
+						struct lacpdu_header *);
+			RTE_VERIFY(lacp->lacpdu.subtype == SLOW_SUBTYPE_LACP);
+
+			/* This is LACP frame so pass it to rx callback.
+			 * Callback is responsible for freeing mbuf.
+			 */
+			mode4->slowrx_cb(slave_id, lacp_pkt);
+		}
+	}
+
+	rte_eal_alarm_set(internals->mode4.update_timeout_us,
+			bond_mode_8023ad_ext_periodic_cb, arg);
+}
diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad.h b/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
index ebd0e93..c196584 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
+++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad.h
@@ -64,6 +64,8 @@  extern "C" {
 #define MARKER_TLV_TYPE_INFO                0x01
 #define MARKER_TLV_TYPE_RESP                0x02
 
+typedef void (*rte_eth_bond_8023ad_ext_slowrx_fn)(uint8_t slave_id, struct rte_mbuf *lacp_pkt);
+
 enum rte_bond_8023ad_selection {
 	UNSELECTED,
 	STANDBY,
@@ -157,6 +159,8 @@  struct rte_eth_bond_8023ad_conf {
 	uint32_t tx_period_ms;
 	uint32_t rx_marker_period_ms;
 	uint32_t update_timeout_ms;
+	rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
+	uint8_t external_sm;
 };
 
 struct rte_eth_bond_8023ad_slave_info {
@@ -219,4 +223,44 @@  rte_eth_bond_8023ad_slave_info(uint8_t port_id, uint8_t slave_id,
 }
 #endif
 
+/**
+ * Configure a slave port to start collecting.
+ *
+ * @param port_id	Bonding device id
+ * @param slave_id	Port id of valid slave.
+ * @param enabled	Non-zero when collection enabled.
+ * @return
+ *   0 - if ok
+ *   -EINVAL if slave is not valid.
+ */
+int
+rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled);
+
+/**
+ * Configure a slave port to start distributing.
+ *
+ * @param port_id	Bonding device id
+ * @param slave_id	Port id of valid slave.
+ * @param enabled	Non-zero when distribution enabled.
+ * @return
+ *   0 - if ok
+ *   -EINVAL if slave is not valid.
+ */
+int
+rte_eth_bond_8023ad_ext_distrib(uint8_t port_id, uint8_t slave_id, int enabled);
+
+/**
+ * LACPDU transmit path for external 802.3ad state machine
+ *
+ * @param port_id	Bonding device id
+ * @param slave_id	Port ID of valid slave device.
+ * @param lacp_pkt	mbuf containing LACPDU.
+ *
+ * @return
+ *   0 on success, negative value otherwise.
+ */
+int
+rte_eth_bond_8023ad_ext_slowtx(uint8_t port_id, uint8_t slave_id,
+		struct rte_mbuf *lacp_pkt);
+
 #endif /* RTE_ETH_BOND_8023AD_H_ */
diff --git a/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h b/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
index 8adee70..9e15ece 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
+++ b/lib/librte_pmd_bond/rte_eth_bond_8023ad_private.h
@@ -173,6 +173,8 @@  struct mode8023ad_private {
 	uint64_t tx_period_timeout;
 	uint64_t rx_marker_timeout;
 	uint64_t update_timeout_us;
+	rte_eth_bond_8023ad_ext_slowrx_fn slowrx_cb;
+	uint8_t external_sm;
 };
 
 /**