On Thu, Feb 23, 2023 at 02:18:59PM +0000, Ferruh Yigit wrote: > On 2/23/2023 4:32 AM, Hemant Agrawal wrote: > > > > On 22-Feb-23 11:25 PM, Honnappa Nagarahalli wrote: > >> -----Original Message----- > >>> From: Richardson, Bruce <bruce.richard...@intel.com> > >>> Sent: Thursday, February 16, 2023 9:05 AM > >>> To: Ferruh Yigit <ferruh.yi...@amd.com>; techbo...@dpdk.org > >>> Cc: Huisong Li <lihuis...@huawei.com>; Chengwen Feng > >>> <fengcheng...@huawei.com> > >>> Subject: RE: MAC address set requires decision > >>> > >>> Alternative suggestions: > >>> > >>> 1. Don't allow "set" of mac address to value already in the list. The > >>> user must > >>> delete the entry manually first before adding it. Similarly, "add" fails > >>> if no > >>> default mac address is set. This ensures consistency by enforcing strict > >>> separation between the default mac address and the extra mac addresses. > >>> You can't have extra addresses without a default, and you can't have > >>> duplicates. > >>> > >>> 2. Always enforce overlap between the two lists - once default mac > >>> address is > >>> set (automatically adding it to the mac addresses list), you can only > >>> replace > >>> the default mac address by using an already-added one to the list. In this > >>> case, the default address is only really an index into the address list, > >>> and no > >>> deletion ever occurs. > >>> > >>> All the solutions below seem rather mixed to me, I'd rather see either > >>> strict > >>> overlap, or strict non-overlap. Both these cases make it that you need > >>> more > >>> calls to do certain tasks, e.g. with #2 to just replace mac address, you > >>> need to > >>> add, set, then delete, but we can always add new, clearly named APIs, to > >>> do > >>> these compound ops. On the plus side, with #2 we could make things doubly > >>> clear by changing the parameter type of "set" to be an index, rather than > >>> explicit mac, to make it clear what is happening, that you are choosing a > >>> default mac from a list of pre-configured options. > >>> > >>> Regards, > >>> /Bruce > > > > Both of the above option seems good. The option #1 above is safe, where > > you are making the mac address set as independent of mac filtering. Also > > making sure that mac filter are not messed up. However, the application > > needs to add error handling now to delete and set. > > > > In the option #2, I assume, it will provide full backward > > compatibility i.e. the ethernet library can take care of the logic and > > application need not to implement anything extra ? If that is the case, > > it seems to be best. > > > > I think #2 is not fully backward compatible, > > Previously set() replaces MAC in list[0], so multiple set() commands end > up with single MAC address. So device will filter only one MAC. > > With #2, after first set(), application will need to add() first, later > set() and del() old MAC, if I understand correctly. > > prev: > set(MAC1) > set(MAC2) > set(MAC3) > > becomes: > set(MAC1) > add(MAC2) > set(MAC2) > del(MAC1) > add(MAC3) > set(MAC3) > del(MAC2) > > > Hence I think this complicates application that wants to just update > default MAC. > > I agree. I tend to think that #1 is the simplest and most backward-compatible of the options. The only case where we end up with different behaviour is the problematic (and already ambiguous) case where one attempts to set a default mac that is already specified in the "alternative" mac list for the card. It keeps all the simple cases as-is.
/Bruce
