From: ext Bill Fischofer [mailto:bill.fischo...@linaro.org] Sent: Tuesday, March 31, 2015 1:13 AM To: Savolainen, Petri (Nokia - FI/Espoo) Cc: LNG ODP Mailman List Subject: Re: [lng-odp] [RFC 8/8] api: packet_io: renamed odp_pktio_promisc_mode_set
On Mon, Mar 30, 2015 at 12:23 PM, Petri Savolainen <petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> wrote: Renamed to odp_pktio_ctrl_promisc_mode(). All interface level control function are prefixed with odp_pktio_ctrl_ to highlight that these operations may not be permitted on all interfaces. For example, virtual functions cannot change interface level settings, only physical functions can. Signed-off-by: Petri Savolainen <petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> --- include/odp/api/packet_io.h | 30 +++++++++++++++++++----------- platform/linux-generic/odp_packet_io.c | 2 +- test/validation/odp_pktio.c | 4 ++-- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/include/odp/api/packet_io.h b/include/odp/api/packet_io.h index dc76270..3229d64 100644 --- a/include/odp/api/packet_io.h +++ b/include/odp/api/packet_io.h @@ -317,17 +317,6 @@ int odp_pktio_ctrl_info(odp_pktio_t pktio, odp_pktio_ctrl_info_t *info); int odp_pktio_mtu(odp_pktio_t pktio); /** - * Enable/Disable promiscuous mode on a packet IO interface. - * - * @param[in] pktio Packet IO handle. - * @param[in] enable 1 to enable, 0 to disable. - * - * @retval 0 on success - * @retval <0 on failure - */ -int odp_pktio_promisc_mode_set(odp_pktio_t pktio, odp_bool_t enable); - -/** * Determine if promiscuous mode is enabled for a packet IO interface. * * @param[in] pktio Packet IO handle. @@ -420,6 +409,25 @@ int odp_pktio_headroom_set(odp_pktio_t pktio, uint32_t headroom); */ uint64_t odp_pktio_to_u64(odp_pktio_t pktio); +/* + * Packet IO interface control + * --------------------------- + * + * Some functions may not be permitted on all interfaces + */ + +/** + * Enable/Disable promiscuous mode on a packet IO interface. + * + * @param[in] pktio Packet IO handle. + * @param[in] enable 1 to enable, 0 to disable. + * + * @retval 0 on success + * @retval <0 on failure + */ +int odp_pktio_ctrl_promisc_mode(odp_pktio_t pktio, odp_bool_t enable); This seems inconsistent with other getters/setters used throughout ODP. Why should these attributes have their own unique syntax instead of odp_pktio_promisc_mode() for the getter and odp_pktio_promisc_mode_set() for setter? The odp_pktio_ctrl_ API would be used to modifying the shared state of the interface (e.g. link up/down, or enable/disable promisc mode). Depending on the interface sharing (e.g. VF vs. PF), some of the API calls may not be supported on the interface the user has opened (a VF). User can call odp_pktio_ctrl_info() to check which ctrl API functions are permitted. odp_pktio_ctrl_info(pktio, &ctrl_info); if (ctrl_info.promisc == 0) { printf(“No permission to set promisc mode\n”); return; } if (odp_pktio_ctrl_promisc_mode(pktio, 1)) printf(“Promisc mode set failed\n”); VS. if (odp_pktio_ctrl_promisc_mode(pktio, 1)) printf(“No permission to set promisc mode, or set failed\n”); This way the potentially not supported functions are grouped together. Other option would be to standadise the return value (-1 fail, -2 not supported) or use errno (-1 fail, errno== ENOSUPPORT). But I think it’s better to group and highlight this class of functions. All odp_xxx_set() functions would remain “supported” always. -Petri
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
