* RE: MAC address set requires decision [not found] ` <DS0PR11MB73094A6B57F581EA716E3DCD97A09@DS0PR11MB7309.namprd11.prod.outlook.com> @ 2023-02-22 17:55 ` Honnappa Nagarahalli 2023-02-23 4:32 ` Hemant Agrawal 0 siblings, 1 reply; 5+ messages in thread From: Honnappa Nagarahalli @ 2023-02-22 17:55 UTC (permalink / raw) To: Richardson, Bruce, Ferruh Yigit, techboard, dev Cc: Huisong Li, Chengwen Feng, nd, nd Hello, Moving this discussion to the dev mailing list as per the request in Techboard meeting today. I could not find a single email with all the responses from Techboard members. So, some of the comments need to be repeated. But this is the base response. Thanks, Honnappa > -----Original Message----- > From: Richardson, Bruce <bruce.richardson@intel.com> > Sent: Thursday, February 16, 2023 9:05 AM > To: Ferruh Yigit <ferruh.yigit@amd.com>; techboard@dpdk.org > Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng > <fengchengwen@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 > > > -----Original Message----- > > From: Ferruh Yigit <ferruh.yigit@amd.com> > > Sent: Thursday, February 16, 2023 2:44 PM > > To: techboard@dpdk.org > > Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng > > <fengchengwen@huawei.com> > > Subject: MAC address set requires decision > > > > Hi Board, > > > > We need a decision on how MAC address set works in DPDK, is it > > possible to vote offline so we can proceed with the patch for this release? > > > > > > Can you please select one of: > > a) Keep current implementation > > b) Proposal 1 > > c) Proposal 2 > > > > Details below, @Huisong feel free to add/correct if needed. > > > > > > > > Background: > > DPDK supports multiple MAC address for MAC filtering. MAC addresses > > are kept in a list, and index 0 is default MAC address. > > > > `rte_eth_dev_default_mac_addr_set()` -> sets default MAC [ set() ] > > `rte_eth_dev_mac_addr_add()` -> adds MAC to list, if no default MAC > > set this adds to index 0 [ add() ] `rte_eth_dev_mac_addr_remove()` -> > > remove MAC from list [ del() ] > > > > > > Problem: > > When a MAC address is already in the list, if set() called, what will > > be the behavior? Like: > > > > add(MAC1) => MAC1 > > add(MAC2) => MAC1, MAC2 > > add(MAC3) => MAC1, MAC2, MAC3 > > set(MAC2) => ??? > > > > > > > > Current code behavior: > > add(MAC1) => MAC1 > > add(MAC2) => MAC1, MAC2 > > add(MAC3) => MAC1, MAC2, MAC3 > > set(MAC2) => MAC2, MAC2, MAC3 > > > > Problem with current behavior: > > - A MAC address is duplicated in list (MAC2), and this leads different > > implementation for different PMDs. Some removes MAC2 filter some not. > > - Can't delete duplicate, because del() tries to delete first MAC it > > finds and since it first finds default MAC address, fails to delete. > > (We can fix del() if desicion to keep this implementation.) > > > > > > > > Proposal 1 (in the patchwork): > > https://patches.dpdk.org/project/dpdk/patch/20230202123625.14975-1- > > lihuisong@huawei.com/ > > > > set(MAC) deletes MAC if it is in the list: > > > > add(MAC1) => MAC1 > > add(MAC2) => MAC1, MAC2 > > add(MAC3) => MAC1, MAC2, MAC3 > > set(MAC2) => MAC2, MAC3 > > set(MAC3) => MAC3 > > > > > > Disagreement on this proposal: > > - It causes implicit delete of MAC addresses in the list, so MAC list > > may shrink with multiple set() calls, this may be confusing > > > > > > > > Proposal 2 (suggested alternative): > > set(MAC) { > > if only_default_mac_exist > > replace_default_mac > > > > if MAC exists in list > > swap MAC and list[0] > > else > > replace_default_mac > > } > > > > Intention here is to prevent implicit delete, swap is just a way to > > keep MAC address in the list, like: > > add(MAC1) => MAC1 > > add(MAC2) => MAC1, MAC2 > > add(MAC3) => MAC1, MAC2, MAC3 > > set(MAC2) => MAC2, MAC1, MAC3 > > set(MAC3) => MAC3, MAC1, MAC2 > > > > Disagreement on this proposal: > > - It is not clear user expects to keep swapped MAC address. > > > > > > Thanks, > > Ferruh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: MAC address set requires decision 2023-02-22 17:55 ` MAC address set requires decision Honnappa Nagarahalli @ 2023-02-23 4:32 ` Hemant Agrawal 2023-02-23 14:18 ` Ferruh Yigit 0 siblings, 1 reply; 5+ messages in thread From: Hemant Agrawal @ 2023-02-23 4:32 UTC (permalink / raw) To: Honnappa Nagarahalli, Richardson, Bruce, Ferruh Yigit, techboard, dev Cc: Huisong Li, Chengwen Feng, nd [-- Attachment #1: Type: text/plain, Size: 5304 bytes --] On 22-Feb-23 11:25 PM, Honnappa Nagarahalli wrote: > -----Original Message----- >> From: Richardson, Bruce<bruce.richardson@intel.com> >> Sent: Thursday, February 16, 2023 9:05 AM >> To: Ferruh Yigit<ferruh.yigit@amd.com>;techboard@dpdk.org >> Cc: Huisong Li<lihuisong@huawei.com>; Chengwen Feng >> <fengchengwen@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,Iassume, 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. Regards Hemant >>> -----Original Message----- >>> From: Ferruh Yigit<ferruh.yigit@amd.com> >>> Sent: Thursday, February 16, 2023 2:44 PM >>> To:techboard@dpdk.org >>> Cc: Huisong Li<lihuisong@huawei.com>; Chengwen Feng >>> <fengchengwen@huawei.com> >>> Subject: MAC address set requires decision >>> >>> Hi Board, >>> >>> We need a decision on how MAC address set works in DPDK, is it >>> possible to vote offline so we can proceed with the patch for this release? >>> >>> >>> Can you please select one of: >>> a) Keep current implementation >>> b) Proposal 1 >>> c) Proposal 2 >>> >>> Details below, @Huisong feel free to add/correct if needed. >>> >>> >>> >>> Background: >>> DPDK supports multiple MAC address for MAC filtering. MAC addresses >>> are kept in a list, and index 0 is default MAC address. >>> >>> `rte_eth_dev_default_mac_addr_set()` -> sets default MAC [ set() ] >>> `rte_eth_dev_mac_addr_add()` -> adds MAC to list, if no default MAC >>> set this adds to index 0 [ add() ] `rte_eth_dev_mac_addr_remove()` -> >>> remove MAC from list [ del() ] >>> >>> >>> Problem: >>> When a MAC address is already in the list, if set() called, what will >>> be the behavior? Like: >>> >>> add(MAC1) => MAC1 >>> add(MAC2) => MAC1, MAC2 >>> add(MAC3) => MAC1, MAC2, MAC3 >>> set(MAC2) => ??? >>> >>> >>> >>> Current code behavior: >>> add(MAC1) => MAC1 >>> add(MAC2) => MAC1, MAC2 >>> add(MAC3) => MAC1, MAC2, MAC3 >>> set(MAC2) => MAC2, MAC2, MAC3 >>> >>> Problem with current behavior: >>> - A MAC address is duplicated in list (MAC2), and this leads different >>> implementation for different PMDs. Some removes MAC2 filter some not. >>> - Can't delete duplicate, because del() tries to delete first MAC it >>> finds and since it first finds default MAC address, fails to delete. >>> (We can fix del() if desicion to keep this implementation.) >>> >>> >>> >>> Proposal 1 (in the patchwork): >>> https://patches.dpdk.org/project/dpdk/patch/20230202123625.14975-1- >>> lihuisong@huawei.com/ >>> >>> set(MAC) deletes MAC if it is in the list: >>> >>> add(MAC1) => MAC1 >>> add(MAC2) => MAC1, MAC2 >>> add(MAC3) => MAC1, MAC2, MAC3 >>> set(MAC2) => MAC2, MAC3 >>> set(MAC3) => MAC3 >>> >>> >>> Disagreement on this proposal: >>> - It causes implicit delete of MAC addresses in the list, so MAC list >>> may shrink with multiple set() calls, this may be confusing >>> >>> >>> >>> Proposal 2 (suggested alternative): >>> set(MAC) { >>> if only_default_mac_exist >>> replace_default_mac >>> >>> if MAC exists in list >>> swap MAC and list[0] >>> else >>> replace_default_mac >>> } >>> >>> Intention here is to prevent implicit delete, swap is just a way to >>> keep MAC address in the list, like: >>> add(MAC1) => MAC1 >>> add(MAC2) => MAC1, MAC2 >>> add(MAC3) => MAC1, MAC2, MAC3 >>> set(MAC2) => MAC2, MAC1, MAC3 >>> set(MAC3) => MAC3, MAC1, MAC2 >>> >>> Disagreement on this proposal: >>> - It is not clear user expects to keep swapped MAC address. >>> >>> >>> Thanks, >>> Ferruh [-- Attachment #2: Type: text/html, Size: 42543 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: MAC address set requires decision 2023-02-23 4:32 ` Hemant Agrawal @ 2023-02-23 14:18 ` Ferruh Yigit 2023-02-23 14:32 ` Bruce Richardson 0 siblings, 1 reply; 5+ messages in thread From: Ferruh Yigit @ 2023-02-23 14:18 UTC (permalink / raw) To: hemant.agrawal, Honnappa Nagarahalli, Richardson, Bruce, techboard, dev Cc: Huisong Li, Chengwen Feng, nd 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.richardson@intel.com> >>> Sent: Thursday, February 16, 2023 9:05 AM >>> To: Ferruh Yigit <ferruh.yigit@amd.com>; techboard@dpdk.org >>> Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng >>> <fengchengwen@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. > > > Regards > > Hemant > > >>>> -----Original Message----- >>>> From: Ferruh Yigit <ferruh.yigit@amd.com> >>>> Sent: Thursday, February 16, 2023 2:44 PM >>>> To: techboard@dpdk.org >>>> Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng >>>> <fengchengwen@huawei.com> >>>> Subject: MAC address set requires decision >>>> >>>> Hi Board, >>>> >>>> We need a decision on how MAC address set works in DPDK, is it >>>> possible to vote offline so we can proceed with the patch for this release? >>>> >>>> >>>> Can you please select one of: >>>> a) Keep current implementation >>>> b) Proposal 1 >>>> c) Proposal 2 >>>> >>>> Details below, @Huisong feel free to add/correct if needed. >>>> >>>> >>>> >>>> Background: >>>> DPDK supports multiple MAC address for MAC filtering. MAC addresses >>>> are kept in a list, and index 0 is default MAC address. >>>> >>>> `rte_eth_dev_default_mac_addr_set()` -> sets default MAC [ set() ] >>>> `rte_eth_dev_mac_addr_add()` -> adds MAC to list, if no default MAC >>>> set this adds to index 0 [ add() ] `rte_eth_dev_mac_addr_remove()` -> >>>> remove MAC from list [ del() ] >>>> >>>> >>>> Problem: >>>> When a MAC address is already in the list, if set() called, what will >>>> be the behavior? Like: >>>> >>>> add(MAC1) => MAC1 >>>> add(MAC2) => MAC1, MAC2 >>>> add(MAC3) => MAC1, MAC2, MAC3 >>>> set(MAC2) => ??? >>>> >>>> >>>> >>>> Current code behavior: >>>> add(MAC1) => MAC1 >>>> add(MAC2) => MAC1, MAC2 >>>> add(MAC3) => MAC1, MAC2, MAC3 >>>> set(MAC2) => MAC2, MAC2, MAC3 >>>> >>>> Problem with current behavior: >>>> - A MAC address is duplicated in list (MAC2), and this leads different >>>> implementation for different PMDs. Some removes MAC2 filter some not. >>>> - Can't delete duplicate, because del() tries to delete first MAC it >>>> finds and since it first finds default MAC address, fails to delete. >>>> (We can fix del() if desicion to keep this implementation.) >>>> >>>> >>>> >>>> Proposal 1 (in the patchwork): >>>> https://patches.dpdk.org/project/dpdk/patch/20230202123625.14975-1- >>>> lihuisong@huawei.com/ >>>> >>>> set(MAC) deletes MAC if it is in the list: >>>> >>>> add(MAC1) => MAC1 >>>> add(MAC2) => MAC1, MAC2 >>>> add(MAC3) => MAC1, MAC2, MAC3 >>>> set(MAC2) => MAC2, MAC3 >>>> set(MAC3) => MAC3 >>>> >>>> >>>> Disagreement on this proposal: >>>> - It causes implicit delete of MAC addresses in the list, so MAC list >>>> may shrink with multiple set() calls, this may be confusing >>>> >>>> >>>> >>>> Proposal 2 (suggested alternative): >>>> set(MAC) { >>>> if only_default_mac_exist >>>> replace_default_mac >>>> >>>> if MAC exists in list >>>> swap MAC and list[0] >>>> else >>>> replace_default_mac >>>> } >>>> >>>> Intention here is to prevent implicit delete, swap is just a way to >>>> keep MAC address in the list, like: >>>> add(MAC1) => MAC1 >>>> add(MAC2) => MAC1, MAC2 >>>> add(MAC3) => MAC1, MAC2, MAC3 >>>> set(MAC2) => MAC2, MAC1, MAC3 >>>> set(MAC3) => MAC3, MAC1, MAC2 >>>> >>>> Disagreement on this proposal: >>>> - It is not clear user expects to keep swapped MAC address. >>>> >>>> >>>> Thanks, >>>> Ferruh ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: MAC address set requires decision 2023-02-23 14:18 ` Ferruh Yigit @ 2023-02-23 14:32 ` Bruce Richardson 2023-03-06 7:01 ` lihuisong (C) 0 siblings, 1 reply; 5+ messages in thread From: Bruce Richardson @ 2023-02-23 14:32 UTC (permalink / raw) To: Ferruh Yigit Cc: hemant.agrawal, Honnappa Nagarahalli, techboard, dev, Huisong Li, Chengwen Feng, nd 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.richardson@intel.com> > >>> Sent: Thursday, February 16, 2023 9:05 AM > >>> To: Ferruh Yigit <ferruh.yigit@amd.com>; techboard@dpdk.org > >>> Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng > >>> <fengchengwen@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: MAC address set requires decision 2023-02-23 14:32 ` Bruce Richardson @ 2023-03-06 7:01 ` lihuisong (C) 0 siblings, 0 replies; 5+ messages in thread From: lihuisong (C) @ 2023-03-06 7:01 UTC (permalink / raw) To: Bruce Richardson, Ferruh Yigit Cc: hemant.agrawal, Honnappa Nagarahalli, techboard, dev, Chengwen Feng, nd 在 2023/2/23 22:32, Bruce Richardson 写道: > 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.richardson@intel.com> >>>>> Sent: Thursday, February 16, 2023 9:05 AM >>>>> To: Ferruh Yigit <ferruh.yigit@amd.com>; techboard@dpdk.org >>>>> Cc: Huisong Li <lihuisong@huawei.com>; Chengwen Feng >>>>> <fengchengwen@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. > Hi all, "The user don't allow "set" of mac address to value already in the list. The user must delete the entry manually first before adding it." Is this the final decision? keep it what it was? Do the ethdev framework prohibit the user from setting MAC address that are already in the list? Or, why cannot the ethdev framework explicitly delete the address first before setting it, like patch [1]? Whether there is a problem in this case is completely guaranteed by the user. If the user forget or don't know, there is a another problem as the commit of patch [1] described. [1] https://patches.dpdk.org/project/dpdk/patch/20230202123625.14975-1-lihuisong@huawei.com/ /Huisong > . ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-06 7:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <d58e359c-dc25-4c6e-b9a9-436bd9ed08af@amd.com>
[not found] ` <DS0PR11MB73094A6B57F581EA716E3DCD97A09@DS0PR11MB7309.namprd11.prod.outlook.com>
2023-02-22 17:55 ` MAC address set requires decision Honnappa Nagarahalli
2023-02-23 4:32 ` Hemant Agrawal
2023-02-23 14:18 ` Ferruh Yigit
2023-02-23 14:32 ` Bruce Richardson
2023-03-06 7:01 ` lihuisong (C)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).