Re: [dpdk-dev] [PATCH v2] net/bonding: add add/remove mac addrs

[Alex Kiselev <alex@therouter.net>]

> From: Chas Williams
>> On Mon, Jun 18, 2018 at 2:58 PM Stephen Hemminger <
>> stephen@networkplumber.org> wrote:

>> > On Mon, 18 Jun 2018 15:27:16 +0300
>> > Alex Kiselev <alex@therouter.net> wrote:
>> >
>> > > +static const struct ether_addr null_mac_addr;
>> > > +
>> > > +/*
>> > > + * Add additional MAC addresses to the slave  */ int
>> > > +slave_add_mac_addresses(struct rte_eth_dev *bonded_eth_dev,
>> > > +             uint16_t slave_port_id) {
>> > > +     int i, ret;
>> > > +     struct ether_addr *mac_addr;
>> > > +
>> > > +     /* add additional MACs to the slave */
>> > > +     for (i = 1; i < BOND_MAX_MAC_ADDRS; i++) {
>> > > +             mac_addr = &bonded_eth_dev->data->mac_addrs[i];
>> > > +             if (is_same_ether_addr(mac_addr, &null_mac_addr))
>> > > +                     break;
>> > > +
>> > > +             ret = rte_eth_dev_mac_addr_add(slave_port_id, mac_addr, 0);
>> > > +             if (ret < 0)
>> > > +                     return ret;
>> > > +     }
>> > > +
>> > > +     return 0;
>> > You need to unwind if adding MAC address to one of the slave devices
>> > worked, and the second one did not.
>> >
Done in patch v3.
Also, I've added the same rollback to bond_ethdev_mac_addr_add().


> Agree.

>> Yes, probably.  But that doesn't help with the new slave problem.  If you add a
>> new slave and it is unable to add all the MAC addresses, what then?  The only
>> reasonable thing might be to put that interface into promiscuous mode.

> I think that in this case the slave should be rejected.
> The reason is that all the slaves should be able to receive\send the same traffic all the time.

>>  At some point you need to draw the line, where is the PMD and where is the
>> application?

>> Thankfully, people tend to enslave the same types of PMDs and the capabilities
>> are generally similar.

> So, I think we need to enforce it in the bonding PMD.


> Please look at the bonding rte_flow design \ RSS conf (also in bonding documentation):

> The main principle in the design is that all the time all the
> slaves able to get\send the same traffic:
> So, a new slave which cannot apply all the current bonding flow rules will be rejected.
> And a new rule which cannot be applied to all the current slaves
> will be rejected and deleted from all the slaves.
> I think it should be the same for mac address adding and all the
> other configurations which affect a traffic receiving\sending.


-- 
Alex