On 02/08/17 09:03, shally verma wrote: > On Wed, Aug 2, 2017 at 1:39 AM, Maxim Uvarov <maxim.uva...@linaro.org> wrote: >> On 08/01/17 15:02, Bill Fischofer wrote:
>>>> int odp_pktio_mac_addr(odp_pktio_t pktio, void *mac_addr, int size); >>>> >>>> /** >>>> + * Set the default MAC address of a packet IO interface. >>>> + * >>>> + * Changes interface default MAC address to the address pointed by >>>> 'mac_addr'. >>>> + * Value of 'size' must be equal to the interface mac address size, which >>>> is >>>> + * specified by 'mac_addr_len' capability. Operation returns failure on >>>> other >>>> + * values of 'size'. MAC address is not changed on failure. >>>> + * >>>> + * @param pktio Packet IO handle >>>> + * @param mac_addr Pointer to MAC address to be set >>>> + * @param size Size of MAC address buffer >>>> + * >>>> + * @return 0 on success >>>> + * @retval <0 on failure >>>> + * >>>> + * @note This API only modifies default MAC address. It doesn’t impact >>>> + * addresses added via call to odp_mac_add_add(). >>>> + */ >>>> +int odp_pktio_mac_addr_set(odp_pktio_t pktio, const uint8_t *mac_addr, >>>> + int size); >>>> + >>>> +/** >>>> + * Add MAC address to a packet IO interface. >>>> + * >>>> + * Adds one or more number of MAC addresses to the given packet IO >>>> interface. >>>> + * Operation returns failure for num > odp_pktio_capablity_t:max_mac_ >>>> addresses. >>>> + * Else return number of mac addresses actually set. >>>> + * MAC addresses are not added on failure. >> >> I think that on falure for set of mac address result is undefined. The >> reason for that you loaded some addresses to chip, some not. So on >> failure you need to get addresses and check what was exactly loaded. >> Even might return -index (negative index) of first failed to load mac. This looks like overloading of return result. I see three viable alternatives: - Try to do as much as possible. Return the array of statuses/errors telling which addresses got propagated to hw and which did not. - Stop on first error. Return amount of addresses processed successfully. - Transaction-like. Either all or none addresses get set. Return 0 on success, -1 on failure. I'd settle for either option 2 or option 3. >> >>>> + * >>>> + * @param pktio Packet IO handle >>>> + * @param mac_addr_tbl Points to an array of MAC addresses to be added >>>> + * @param num Number of MAC addresses to be added >>>> + * >>>> + * @return Number of MAC addresses actually set >> >> Nope. 0 on success. That is silly to expect partial load. >> >>>> + * @retval <0 on failure >>>> + * >>>> + * @note If number returns < number originally passed, application needs >>>> + * to verify odp_pktio_mac_addr_t:status parameter of each MAC address to >>>> + * confirm whether the corresponding MAC address is added successfully or >>>> not. >> >> load should be stopped on first error. >> > > Shally - One summary to address all responses around output parameter > "status". > > Consider following use case: > user added mac addresses @ index 0, 1, 2, 3, 4 > user removed #1 and #3 --> now we have hole at #1 and #3 > user try to add #1 #2 and #3 (though he shouldn't pass #2 but still > passes) --> here only #1 would be set but not #2 (as its already > occupied with valid entry). So what is the expectations? should API > return at #2 Or continue to #3(as 3 is available to set)? > > if we want to enforce a rule that all indexes up to 'requested' should > be empty / removed, then we can get rid of "status" as output param. > Else, we can allow this flexibility to implementation to go ahead and > consume as much as possible > and update "status" w.r.t each entry so that app could know which of > 'n' returned entries have been set. > > I think we can debate further of this based on above example. Any > further responses to the following feedback will be more clear once we > understand expectations to scenario above. You can have an array of statuses as output parameter. -- With best wishes Dmitry
