Hi Eric,

<snip>

quoted

quoted

quoted

@@ -157,6 +159,7 @@ 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;
  };

This still is a likely an ABI break, previously discussed around here:
http://dpdk.org/ml/archives/dev/2015-November/027321.html

It might not be embedded anywhere in DPDK codebase, but there's no
telling what others might have done with it (have an array of them,
embed in other structs etc).

Also ultimately ABI compatibility goes both ways: when the library
soname does not change then an application is free to assume both
downgrading and upgrading are safe. In this case, upgrading *might*
be okay, downgrading certainly is not. So by that measure it definitely is

an ABI break.

quoted

quoted

[...]

quoted

diff --git a/drivers/net/bonding/rte_eth_bond_version.map

b/drivers/net/bonding/rte_eth_bond_version.map
index 22bd920..33d73ff 100644

--- a/drivers/net/bonding/rte_eth_bond_version.map
+++ b/drivers/net/bonding/rte_eth_bond_version.map

@@ -27,3 +27,9 @@ DPDK_2.1 {
  	rte_eth_bond_free;

  } DPDK_2.0;
+
+DPDK_2.2 {
+	rte_eth_bond_8023ad_ext_collect;
+	rte_eth_bond_8023ad_ext_distrib;
+	rte_eth_bond_8023ad_ext_slowtx;
+} DPDK_2.1;

These symbols are not part of DPDK 2.2, the version here is wrong.
Technically it would not actually matter much but better not to
confuse things unnecessarily.

	- Panu -

It looks like Panu's points are valid, a V3 of this patch set which takes care of

these issues will be needed.

quoted

Patches 1/6, 5/6 and 6/6 of the patch set are bug fixes, so each patch

should contain a fixes line.

quoted

Patches 2/6, 3/6 and 4/6 are a new feature, the release notes should be

updated for this feature.

quoted

Could I suggest splitting the patch set into two patch sets, a bug fix patch

set and a new feature patch set.

quoted

Regards,

Bernard.

Bernard, a v3 is on the way.  I included only things that are fixes, but the
patch set doesn't quite match the set of patch numbers you listed above.
1/6 (bond: use existing enslaved device queues) is an improvement, but
doesn't really fix anything that was broken, so I left that out.
2/6 (bond mode 4: copy entire config structure) and 3/6 (bond mode 4:
do not ignore multicast) fix bugs and are included.


Eric

Thanks for the V3 patchset and clarifying which patches in the V2 patchset were fixes.

Regards,

Bernard.