| Message ID | 1415729700-23420-1-git-send-email-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Nov 11, 2014 at 12:14 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Implement pktio mtu functions: > odp_pktio_mtu() to get mtu value; > odp_pktio_set_mtu() to set mtu value. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > Reviewed-by: Bill Fischofer <bill.fischofer@linaro.org> > --- > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ > platform/linux-generic/odp_packet_io.c | 50 > ++++++++++++++++++++++ > 3 files changed, 74 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h > b/platform/linux-generic/include/api/odp_packet_io.h > index 360636d..b784705 100644 > --- a/platform/linux-generic/include/api/odp_packet_io.h > +++ b/platform/linux-generic/include/api/odp_packet_io.h > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, > odp_pktio_t id); > */ > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); > > +/* > + * Configure the MTU for a packet IO interface. > + * > + * @param[in] id ODP packet IO handle. > + * @param[in] mtu The MTU to be applied. > + * > + * @return 0 on success, -1 on error. > + */ > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); > + > +/* > + * Get the currently configured MTU of a packet IO interface. > + * > + * @param[in] id ODP packet IO handle. > + * @param[out] mtu Pointer to location in which to store the MTU value. > + * > + * @return 0 on success, -1 on error. > + */ > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); > + > /** > * @} > */ > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h > b/platform/linux-generic/include/odp_packet_io_internal.h > index 23633ed..ff73970 100644 > --- a/platform/linux-generic/include/odp_packet_io_internal.h > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > @@ -21,6 +21,8 @@ extern "C" { > #include <odp_spinlock.h> > #include <odp_packet_socket.h> > > +#include <linux/if.h> > + > /** > * Packet IO types > */ > @@ -38,6 +40,8 @@ struct pktio_entry { > odp_pktio_type_t type; /**< pktio type */ > pkt_sock_t pkt_sock; /**< using socket API for IO */ > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API for IO > */ > + char name[IFNAMSIZ]; /**< name of pktio provided to > + pktio_open() */ > }; > > typedef union { > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > index f35193f..20fe10a 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -20,6 +20,7 @@ > #include <odp_debug.h> > > #include <string.h> > +#include <sys/ioctl.h> > > typedef struct { > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, > odp_buffer_pool_t pool) > return ODP_PKTIO_INVALID; > > done: > + strncpy(pktio_entry->s.name, dev, IFNAMSIZ); > unlock_entry(pktio_entry); > return id; > } > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, > odp_buffer_hdr_t *buf_hdr[], int num) > > return nbr; > } > + > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) > +{ > + pktio_entry_t *pktio_entry = get_entry(id); > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = pktio_entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > + ifr.ifr_mtu = mtu; > + > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); > + if (ret != 0) { > + ODP_ERR("ioctl SIOCSIFMTU error\n"); > + return -1; > + } > + > + return 0; > +} > + > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) > +{ > + pktio_entry_t *pktio_entry = get_entry(id); > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = pktio_entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > + > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); > + if (ret != 0) { > + ODP_ERR("ioctl SIOCGIFMTU error\n"); > + return -1; > + } > + > + *mtu = ifr.ifr_mtu; > + return 0; > +} > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
Drop linux-generic because we only have linux-generic and say something like: "pktio: add MTU manipulation functions" On 2014-11-11 21:14, Maxim Uvarov wrote: > Implement pktio mtu functions: > odp_pktio_mtu() to get mtu value; > odp_pktio_set_mtu() to set mtu value. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ > platform/linux-generic/odp_packet_io.c | 50 ++++++++++++++++++++++ > 3 files changed, 74 insertions(+) > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h > index 360636d..b784705 100644 > --- a/platform/linux-generic/include/api/odp_packet_io.h > +++ b/platform/linux-generic/include/api/odp_packet_io.h > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, odp_pktio_t id); > */ > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); > > +/* > + * Configure the MTU for a packet IO interface. Is this description enough for a new user to understand what will occur? > + * > + * @param[in] id ODP packet IO handle. hdl should be the standard way for ODP to specify the handle. > + * @param[in] mtu The MTU to be applied. The value of MTU that the interface will be configured to use. > + * > + * @return 0 on success, -1 on error. @retval 0 <reason> @retval -1 <reason(s)> Elaborate on what causes an error, if it's not possible to provide a good reason for -1 should this be a void return? > + */ > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); > + > +/* > + * Get the currently configured MTU of a packet IO interface. Is this description enough for a new user to understand what will occur? > + * > + * @param[in] id ODP packet IO handle. Does not line up hdl should be the standard way for ODP to specify the handle. > + * @param[out] mtu Pointer to location in which to store the MTU value. > + * > + * @return 0 on success, -1 on error. > + */ > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); unsigned int *odp_pktio_mtu(odp_pktio_t id); @retval mtu on succes @retval NULL on error <list all the errors> > + > /** > * @} > */ > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h > index 23633ed..ff73970 100644 > --- a/platform/linux-generic/include/odp_packet_io_internal.h > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > @@ -21,6 +21,8 @@ extern "C" { > #include <odp_spinlock.h> > #include <odp_packet_socket.h> > > +#include <linux/if.h> > + > /** > * Packet IO types > */ > @@ -38,6 +40,8 @@ struct pktio_entry { > odp_pktio_type_t type; /**< pktio type */ > pkt_sock_t pkt_sock; /**< using socket API for IO */ > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API for IO */ > + char name[IFNAMSIZ]; /**< name of pktio provided to > + pktio_open() */ Does not line up. Cheers, Anders > }; > > typedef union { > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index f35193f..20fe10a 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -20,6 +20,7 @@ > #include <odp_debug.h> > > #include <string.h> > +#include <sys/ioctl.h> > > typedef struct { > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool) > return ODP_PKTIO_INVALID; > > done: > + strncpy(pktio_entry->s.name, dev, IFNAMSIZ); > unlock_entry(pktio_entry); > return id; > } > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num) > > return nbr; > } > + > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) > +{ > + pktio_entry_t *pktio_entry = get_entry(id); > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = pktio_entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > + ifr.ifr_mtu = mtu; > + > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); > + if (ret != 0) { > + ODP_ERR("ioctl SIOCSIFMTU error\n"); > + return -1; > + } > + > + return 0; > +} > + > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) > +{ > + pktio_entry_t *pktio_entry = get_entry(id); > + int sockfd; > + struct ifreq ifr; > + int ret; > + > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > + else > + sockfd = pktio_entry->s.pkt_sock.sockfd; > + > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > + > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); > + if (ret != 0) { > + ODP_ERR("ioctl SIOCGIFMTU error\n"); > + return -1; > + } > + > + *mtu = ifr.ifr_mtu; > + return 0; > +} > -- > 1.8.5.1.163.gd7aced9 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On Tue, Nov 11, 2014 at 2:28 PM, Anders Roxell <anders.roxell@linaro.org> wrote: > Drop linux-generic because we only have linux-generic and say something > like: > "pktio: add MTU manipulation functions" > > On 2014-11-11 21:14, Maxim Uvarov wrote: > > Implement pktio mtu functions: > > odp_pktio_mtu() to get mtu value; > > odp_pktio_set_mtu() to set mtu value. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ > > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ > > platform/linux-generic/odp_packet_io.c | 50 > ++++++++++++++++++++++ > > 3 files changed, 74 insertions(+) > > > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h > b/platform/linux-generic/include/api/odp_packet_io.h > > index 360636d..b784705 100644 > > --- a/platform/linux-generic/include/api/odp_packet_io.h > > +++ b/platform/linux-generic/include/api/odp_packet_io.h > > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, > odp_pktio_t id); > > */ > > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); > > > > +/* > > + * Configure the MTU for a packet IO interface. > > Is this description enough for a new user to understand > what will occur? > I'd think so. MTU is a pretty standard bit of interface metadata. > > > + * > > + * @param[in] id ODP packet IO handle. > > hdl should be the standard way for ODP to specify the handle. > > > + * @param[in] mtu The MTU to be applied. > > The value of MTU that the interface will be configured to use. > > > + * > > + * @return 0 on success, -1 on error. > > @retval 0 <reason> > @retval -1 <reason(s)> > > Elaborate on what causes an error, if it's not possible to provide a > good reason for -1 should this be a void return? > The ioctl may fail. Wouldn't the application want to know that? For example the range of valid MTUs will vary by device and ODP can't be expected to know that. It just calls the ioctl and Linux tells whether that's OK. Since linux will set errno on ioctl failure, -1 is just following the standard ODP error return protocol. > > > + */ > > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); > > + > > +/* > > + * Get the currently configured MTU of a packet IO interface. > > Is this description enough for a new user to understand > what will occur? > Again, yes. > > > + * > > + * @param[in] id ODP packet IO handle. > > Does not line up > > hdl should be the standard way for ODP to specify the handle. > > > > + * @param[out] mtu Pointer to location in which to store the MTU value. > > + * > > + * @return 0 on success, -1 on error. > > + */ > > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); > > unsigned int *odp_pktio_mtu(odp_pktio_t id); > > @retval mtu on succes > @retval NULL on error <list all the errors> > No, this routine has an output parameter. This is just following standard ODP RC conventions for reporting success/failure as an int function return value. NULL can only be assigned to a pointer, not an int. Regarding the errors, again the ioctl will set errno so if the caller want's to know why it failed it can read that. It's system-dependent since each interface may be slightly different. > > > + > > /** > > * @} > > */ > > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h > b/platform/linux-generic/include/odp_packet_io_internal.h > > index 23633ed..ff73970 100644 > > --- a/platform/linux-generic/include/odp_packet_io_internal.h > > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > > @@ -21,6 +21,8 @@ extern "C" { > > #include <odp_spinlock.h> > > #include <odp_packet_socket.h> > > > > +#include <linux/if.h> > > + > > /** > > * Packet IO types > > */ > > @@ -38,6 +40,8 @@ struct pktio_entry { > > odp_pktio_type_t type; /**< pktio type */ > > pkt_sock_t pkt_sock; /**< using socket API for IO */ > > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API for IO > */ > > + char name[IFNAMSIZ]; /**< name of pktio provided to > > + pktio_open() */ > > Does not line up. > Doesn't checkpatch catch actual style errors? I'm assuming this is checkpatch clean. If it is that should be sufficient. If it's not, then it should be made checkpatch-clean before posting it. > > Cheers, > Anders > > > }; > > > > typedef union { > > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > > index f35193f..20fe10a 100644 > > --- a/platform/linux-generic/odp_packet_io.c > > +++ b/platform/linux-generic/odp_packet_io.c > > @@ -20,6 +20,7 @@ > > #include <odp_debug.h> > > > > #include <string.h> > > +#include <sys/ioctl.h> > > > > typedef struct { > > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, > odp_buffer_pool_t pool) > > return ODP_PKTIO_INVALID; > > > > done: > > + strncpy(pktio_entry->s.name, dev, IFNAMSIZ); > > unlock_entry(pktio_entry); > > return id; > > } > > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, > odp_buffer_hdr_t *buf_hdr[], int num) > > > > return nbr; > > } > > + > > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) > > +{ > > + pktio_entry_t *pktio_entry = get_entry(id); > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = pktio_entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > > + ifr.ifr_mtu = mtu; > > + > > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); > > + if (ret != 0) { > > + ODP_ERR("ioctl SIOCSIFMTU error\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) > > +{ > > + pktio_entry_t *pktio_entry = get_entry(id); > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = pktio_entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); > > + > > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); > > + if (ret != 0) { > > + ODP_ERR("ioctl SIOCGIFMTU error\n"); > > + return -1; > > + } > > + > > + *mtu = ifr.ifr_mtu; > > + return 0; > > +} > > -- > > 1.8.5.1.163.gd7aced9 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp > > -- > Anders Roxell > anders.roxell@linaro.org > M: +46 709 71 42 85 | IRC: roxell > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 11 November 2014 15:41, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Tue, Nov 11, 2014 at 2:28 PM, Anders Roxell <anders.roxell@linaro.org> > wrote: >> >> Drop linux-generic because we only have linux-generic and say something >> like: >> "pktio: add MTU manipulation functions" >> >> On 2014-11-11 21:14, Maxim Uvarov wrote: >> > Implement pktio mtu functions: >> > odp_pktio_mtu() to get mtu value; >> > odp_pktio_set_mtu() to set mtu value. >> > >> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> >> > --- >> > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ >> > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ >> > platform/linux-generic/odp_packet_io.c | 50 >> > ++++++++++++++++++++++ >> > 3 files changed, 74 insertions(+) >> > >> > diff --git a/platform/linux-generic/include/api/odp_packet_io.h >> > b/platform/linux-generic/include/api/odp_packet_io.h >> > index 360636d..b784705 100644 >> > --- a/platform/linux-generic/include/api/odp_packet_io.h >> > +++ b/platform/linux-generic/include/api/odp_packet_io.h >> > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, >> > odp_pktio_t id); >> > */ >> > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); >> > >> > +/* >> > + * Configure the MTU for a packet IO interface. >> >> Is this description enough for a new user to understand >> what will occur? > > > I'd think so. MTU is a pretty standard bit of interface metadata. > >> >> >> > + * >> > + * @param[in] id ODP packet IO handle. >> >> hdl should be the standard way for ODP to specify the handle. >> >> > + * @param[in] mtu The MTU to be applied. >> >> The value of MTU that the interface will be configured to use. >> >> > + * >> > + * @return 0 on success, -1 on error. >> >> @retval 0 <reason> >> @retval -1 <reason(s)> >> >> Elaborate on what causes an error, if it's not possible to provide a >> good reason for -1 should this be a void return? > > > The ioctl may fail. Wouldn't the application want to know that? For > example the range of valid MTUs will vary by device and ODP can't be > expected to know that. It just calls the ioctl and Linux tells whether > that's OK. Since linux will set errno on ioctl failure, -1 is just > following the standard ODP error return protocol. I think there are errors that can be listed in this case I don't think just saying -1 on error is enough for the documentation. I can't write unit tests for that, what cases am I testing ? Worse on non linux implementations you need to know exactly when to return error, saying errors match those thrown by linux ioctl is probably not accurate enough. we need @retval -1 requested MTU is not supported by the interface - if there is an errno that matters we need to list that too I guess > >> >> >> > + */ >> > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); >> > + >> > +/* >> > + * Get the currently configured MTU of a packet IO interface. >> >> Is this description enough for a new user to understand >> what will occur? > > > Again, yes. > >> >> >> > + * >> > + * @param[in] id ODP packet IO handle. >> >> Does not line up >> >> hdl should be the standard way for ODP to specify the handle. I agree with this, we don't want synonyms, a handle (hdl) should be a handle so that the ODP APIs all look and feel the same, currently we don't have this. >> >> >> > + * @param[out] mtu Pointer to location in which to store the MTU >> > value. >> > + * >> > + * @return 0 on success, -1 on error. >> > + */ >> > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); >> >> unsigned int *odp_pktio_mtu(odp_pktio_t id); >> >> @retval mtu on succes >> @retval NULL on error <list all the errors> > > > No, this routine has an output parameter. This is just following standard > ODP RC conventions for reporting success/failure as an int function return > value. NULL can only be assigned to a pointer, not an int. Regarding the > errors, again the ioctl will set errno so if the caller want's to know why > it failed it can read that. It's system-dependent since each interface may > be slightly different. > The doc says this http://docs.opendataplane.org/arch/html/api_guide_lines.html maybe there is something in this discussion we can capture there ? In this case I think it is less cumbersome to return the value you are looking for rather than have to pass a pointer to an int and populate that with the result, -1 could still be error, not sure that we disallow that in the doc (yet :) ) Unless we clearly always pass in a pointer for such things as part of our look and feel. I think Anders was eating spicy sausages when he put an int pointer however, it is int :) >> >> >> > + >> > /** >> > * @} >> > */ >> > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> > b/platform/linux-generic/include/odp_packet_io_internal.h >> > index 23633ed..ff73970 100644 >> > --- a/platform/linux-generic/include/odp_packet_io_internal.h >> > +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> > @@ -21,6 +21,8 @@ extern "C" { >> > #include <odp_spinlock.h> >> > #include <odp_packet_socket.h> >> > >> > +#include <linux/if.h> >> > + >> > /** >> > * Packet IO types >> > */ >> > @@ -38,6 +40,8 @@ struct pktio_entry { >> > odp_pktio_type_t type; /**< pktio type */ >> > pkt_sock_t pkt_sock; /**< using socket API for IO */ >> > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API for IO >> > */ >> > + char name[IFNAMSIZ]; /**< name of pktio provided to >> > + pktio_open() */ >> >> Does not line up. > > > Doesn't checkpatch catch actual style errors? I'm assuming this is > checkpatch clean. If it is that should be sufficient. If it's not, then it > should be made checkpatch-clean before posting it. Checkpatch does a lot, but a lot can pass it that really jars viewers eyeballs. I completely agree however that to submit a patch it much be checkpatch clean, but I think there is scope for reviewers to ask for minor compatible changes for example to ask that lists line up, things like that. > >> >> >> Cheers, >> Anders >> >> > }; >> > >> > typedef union { >> > diff --git a/platform/linux-generic/odp_packet_io.c >> > b/platform/linux-generic/odp_packet_io.c >> > index f35193f..20fe10a 100644 >> > --- a/platform/linux-generic/odp_packet_io.c >> > +++ b/platform/linux-generic/odp_packet_io.c >> > @@ -20,6 +20,7 @@ >> > #include <odp_debug.h> >> > >> > #include <string.h> >> > +#include <sys/ioctl.h> >> > >> > typedef struct { >> > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; >> > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, >> > odp_buffer_pool_t pool) >> > return ODP_PKTIO_INVALID; >> > >> > done: >> > + strncpy(pktio_entry->s.name, dev, IFNAMSIZ); >> > unlock_entry(pktio_entry); >> > return id; >> > } >> > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, >> > odp_buffer_hdr_t *buf_hdr[], int num) >> > >> > return nbr; >> > } >> > + >> > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) >> > +{ >> > + pktio_entry_t *pktio_entry = get_entry(id); >> > + int sockfd; >> > + struct ifreq ifr; >> > + int ret; >> > + >> > + if (pktio_entry->s.pkt_sock_mmap.sockfd) >> > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; >> > + else >> > + sockfd = pktio_entry->s.pkt_sock.sockfd; >> > + >> > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); >> > + ifr.ifr_mtu = mtu; >> > + >> > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); >> > + if (ret != 0) { >> > + ODP_ERR("ioctl SIOCSIFMTU error\n"); >> > + return -1; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) >> > +{ >> > + pktio_entry_t *pktio_entry = get_entry(id); >> > + int sockfd; >> > + struct ifreq ifr; >> > + int ret; >> > + >> > + if (pktio_entry->s.pkt_sock_mmap.sockfd) >> > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; >> > + else >> > + sockfd = pktio_entry->s.pkt_sock.sockfd; >> > + >> > + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); >> > + >> > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); >> > + if (ret != 0) { >> > + ODP_ERR("ioctl SIOCGIFMTU error\n"); >> > + return -1; >> > + } >> > + >> > + *mtu = ifr.ifr_mtu; >> > + return 0; >> > +} >> > -- >> > 1.8.5.1.163.gd7aced9 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> -- >> Anders Roxell >> anders.roxell@linaro.org >> M: +46 709 71 42 85 | IRC: roxell >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
I wonder why these question were risen now. In patch review, not in arch review :) But summarizing everything: 1. Patches are checkpatch clean. 2. Minor style check for comment - agree will correct it. 3. Requested change 'odp_pktio_t id' to 'odp_pktio_t hdl' is not correct. That is subject for other patch. Code has to look same for all function. Everywhere we use 'odp_pktio_t id'. Run: fgrep -r odp_pktio_t ./platform/linux-generic 4. Return codes. I followed return codes as API proposed. Please note that I used ioctl, but I could do it in other way. So it is only implementation. Inside implementation function I added ODP_ERR. If you need specific reason run strace on your application, that is very common for linux apps. If we decided to add more error codes to odp, then we can add. Like ERROR_MTU_TOO_SMALL, ERROR_MTU_TOO_BIG. But that has to go to arch first. And I would like to have it in separate patch. To discuss with vendors if that returns codes are valuable and if they can really return them. Maxim. On 11/11/2014 11:41 PM, Bill Fischofer wrote: > > > On Tue, Nov 11, 2014 at 2:28 PM, Anders Roxell > <anders.roxell@linaro.org <mailto:anders.roxell@linaro.org>> wrote: > > Drop linux-generic because we only have linux-generic and say > something > like: > "pktio: add MTU manipulation functions" > > On 2014-11-11 21:14, Maxim Uvarov wrote: > > Implement pktio mtu functions: > > odp_pktio_mtu() to get mtu value; > > odp_pktio_set_mtu() to set mtu value. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> > > --- > > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ > > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ > > platform/linux-generic/odp_packet_io.c | 50 ++++++++++++++++++++++ > > 3 files changed, 74 insertions(+) > > > > diff --git a/platform/linux-generic/include/api/odp_packet_io.h > b/platform/linux-generic/include/api/odp_packet_io.h > > index 360636d..b784705 100644 > > --- a/platform/linux-generic/include/api/odp_packet_io.h > > +++ b/platform/linux-generic/include/api/odp_packet_io.h > > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, > odp_pktio_t id); > > */ > > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); > > > > +/* > > + * Configure the MTU for a packet IO interface. > > Is this description enough for a new user to understand > what will occur? > > > I'd think so. MTU is a pretty standard bit of interface metadata. > > > > + * > > + * @param[in] id ODP packet IO handle. > > hdl should be the standard way for ODP to specify the handle. > > > + * @param[in] mtu The MTU to be applied. > > The value of MTU that the interface will be configured to use. > > > + * > > + * @return 0 on success, -1 on error. > > @retval 0 <reason> > @retval -1 <reason(s)> > > Elaborate on what causes an error, if it's not possible to provide a > good reason for -1 should this be a void return? > > > The ioctl may fail. Wouldn't the application want to know that? For > example the range of valid MTUs will vary by device and ODP can't be > expected to know that. It just calls the ioctl and Linux tells > whether that's OK. Since linux will set errno on ioctl failure, -1 is > just following the standard ODP error return protocol. > > > > + */ > > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); > > + > > +/* > > + * Get the currently configured MTU of a packet IO interface. > > Is this description enough for a new user to understand > what will occur? > > > Again, yes. > > > > + * > > + * @param[in] id ODP packet IO handle. > > Does not line up > > hdl should be the standard way for ODP to specify the handle. > > > > + * @param[out] mtu Pointer to location in which to store the > MTU value. > > + * > > + * @return 0 on success, -1 on error. > > + */ > > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); > > unsigned int *odp_pktio_mtu(odp_pktio_t id); > > @retval mtu on succes > @retval NULL on error <list all the errors> > > > No, this routine has an output parameter. This is just following > standard ODP RC conventions for reporting success/failure as an int > function return value. NULL can only be assigned to a pointer, not an > int. Regarding the errors, again the ioctl will set errno so if the > caller want's to know why it failed it can read that. It's > system-dependent since each interface may be slightly different. > > > > + > > /** > > * @} > > */ > > diff --git > a/platform/linux-generic/include/odp_packet_io_internal.h > b/platform/linux-generic/include/odp_packet_io_internal.h > > index 23633ed..ff73970 100644 > > --- a/platform/linux-generic/include/odp_packet_io_internal.h > > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > > @@ -21,6 +21,8 @@ extern "C" { > > #include <odp_spinlock.h> > > #include <odp_packet_socket.h> > > > > +#include <linux/if.h> > > + > > /** > > * Packet IO types > > */ > > @@ -38,6 +40,8 @@ struct pktio_entry { > > odp_pktio_type_t type; /**< pktio type */ > > pkt_sock_t pkt_sock; /**< using socket API for > IO */ > > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API > for IO */ > > + char name[IFNAMSIZ]; /**< name of pktio provided to > > + pktio_open() */ > > Does not line up. > > > Doesn't checkpatch catch actual style errors? I'm assuming this is > checkpatch clean. If it is that should be sufficient. If it's not, > then it should be made checkpatch-clean before posting it. > > > Cheers, > Anders > > > }; > > > > typedef union { > > diff --git a/platform/linux-generic/odp_packet_io.c > b/platform/linux-generic/odp_packet_io.c > > index f35193f..20fe10a 100644 > > --- a/platform/linux-generic/odp_packet_io.c > > +++ b/platform/linux-generic/odp_packet_io.c > > @@ -20,6 +20,7 @@ > > #include <odp_debug.h> > > > > #include <string.h> > > +#include <sys/ioctl.h> > > > > typedef struct { > > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; > > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, > odp_buffer_pool_t pool) > > return ODP_PKTIO_INVALID; > > > > done: > > + strncpy(pktio_entry->s.name <http://s.name>, dev, IFNAMSIZ); > > unlock_entry(pktio_entry); > > return id; > > } > > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, > odp_buffer_hdr_t *buf_hdr[], int num) > > > > return nbr; > > } > > + > > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) > > +{ > > + pktio_entry_t *pktio_entry = get_entry(id); > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = pktio_entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, pktio_entry->s.name <http://s.name>, > IFNAMSIZ); > > + ifr.ifr_mtu = mtu; > > + > > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); > > + if (ret != 0) { > > + ODP_ERR("ioctl SIOCSIFMTU error\n"); > > + return -1; > > + } > > + > > + return 0; > > +} > > + > > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) > > +{ > > + pktio_entry_t *pktio_entry = get_entry(id); > > + int sockfd; > > + struct ifreq ifr; > > + int ret; > > + > > + if (pktio_entry->s.pkt_sock_mmap.sockfd) > > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; > > + else > > + sockfd = pktio_entry->s.pkt_sock.sockfd; > > + > > + strncpy(ifr.ifr_name, pktio_entry->s.name <http://s.name>, > IFNAMSIZ); > > + > > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); > > + if (ret != 0) { > > + ODP_ERR("ioctl SIOCGIFMTU error\n"); > > + return -1; > > + } > > + > > + *mtu = ifr.ifr_mtu; > > + return 0; > > +} > > -- > > 1.8.5.1.163.gd7aced9 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > > http://lists.linaro.org/mailman/listinfo/lng-odp > > -- > Anders Roxell > anders.roxell@linaro.org <mailto:anders.roxell@linaro.org> > M: +46 709 71 42 85 <tel:%2B46%20709%2071%2042%2085> | IRC: roxell > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 12 November 2014 07:54, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > I wonder why these question were risen now. In patch review, not in arch > review :) > But summarizing everything: > > 1. Patches are checkpatch clean. > 2. Minor style check for comment - agree will correct it. > 3. Requested change 'odp_pktio_t id' to 'odp_pktio_t hdl' is not correct. > That is subject for other patch. > Code has to look same for all function. Everywhere we use 'odp_pktio_t id'. > Run: fgrep -r odp_pktio_t ./platform/linux-generic > 4. Return codes. I followed return codes as API proposed. Yes agreed and universally the documentation has not defined the error cases making meaningful testing of the 1.0 spec virtually impossible. <- my key point - my rational follows Given that, we must add information to the arch docs as you say below, the correct location is the definition of this API in its header file. The arch docs are stale as soon as an API becomes code that is why when it is built, the arch doc <http://docs.opendataplane.org/arch/html/group__odp__packet__io.html> links to the API docs for this information. In this case there is no arch doc for this API so we need to just add it to the best of our ability in the header file and it will become part of the arch doc and its specification will be testable, linux-generic is is primarily the reference for odp 1.0 not a performance platform. With this implementation you have added an obvious testable error case, if we don't specify it ODP becomes a specification with undefined return behavior. I agree some platforms may have additional error cases (we may add them as they turn up) but an implementer may wish to rely on the defined behavior, consider a contrived case of the general problem // find biggest MTU that is working for this packet_io mtu =0 mtu_status = odp_pktio_set_mtu( id, mtu) while (mtu_status != ok) { mtu++; mtu_status = odp_pktio_set_mtu( id, mtu) } This is not possible for an api that doesn't specify what a bad MTU value will do, it may call abort! Compare our docs for this function and an admittedly more complex linux read - the open group list what will cause an error, they describe how an API is actually supposed to react http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html. Please note that I > used ioctl, but I could do it in other way. > So it is only implementation. Inside implementation function I added > ODP_ERR. If you need specific reason > run strace on your application, that is very common for linux apps. > If we decided to add more error codes to odp, then we can add. Like > ERROR_MTU_TOO_SMALL, ERROR_MTU_TOO_BIG. > But that has to go to arch first. And I would like to have it in separate > patch. To discuss with vendors if that returns codes > are valuable and if they can really return them. > > Maxim. > > On 11/11/2014 11:41 PM, Bill Fischofer wrote: >> >> >> >> On Tue, Nov 11, 2014 at 2:28 PM, Anders Roxell <anders.roxell@linaro.org >> <mailto:anders.roxell@linaro.org>> wrote: >> >> Drop linux-generic because we only have linux-generic and say >> something >> like: >> "pktio: add MTU manipulation functions" >> >> On 2014-11-11 21:14, Maxim Uvarov wrote: >> > Implement pktio mtu functions: >> > odp_pktio_mtu() to get mtu value; >> > odp_pktio_set_mtu() to set mtu value. >> > >> > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> >> >> > --- >> > platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ >> > .../linux-generic/include/odp_packet_io_internal.h | 4 ++ >> > platform/linux-generic/odp_packet_io.c | 50 ++++++++++++++++++++++ >> > 3 files changed, 74 insertions(+) >> > >> > diff --git a/platform/linux-generic/include/api/odp_packet_io.h >> b/platform/linux-generic/include/api/odp_packet_io.h >> > index 360636d..b784705 100644 >> > --- a/platform/linux-generic/include/api/odp_packet_io.h >> > +++ b/platform/linux-generic/include/api/odp_packet_io.h >> > @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, >> odp_pktio_t id); >> > */ >> > odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); >> > >> > +/* >> > + * Configure the MTU for a packet IO interface. >> >> Is this description enough for a new user to understand >> what will occur? >> >> >> I'd think so. MTU is a pretty standard bit of interface metadata. >> >> >> > + * >> > + * @param[in] id ODP packet IO handle. >> >> hdl should be the standard way for ODP to specify the handle. >> >> > + * @param[in] mtu The MTU to be applied. >> >> The value of MTU that the interface will be configured to use. >> >> > + * >> > + * @return 0 on success, -1 on error. >> >> @retval 0 <reason> >> @retval -1 <reason(s)> >> >> Elaborate on what causes an error, if it's not possible to provide a >> good reason for -1 should this be a void return? >> >> >> The ioctl may fail. Wouldn't the application want to know that? For >> example the range of valid MTUs will vary by device and ODP can't be >> expected to know that. It just calls the ioctl and Linux tells whether >> that's OK. Since linux will set errno on ioctl failure, -1 is just >> following the standard ODP error return protocol. >> >> >> > + */ >> > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); >> > + >> > +/* >> > + * Get the currently configured MTU of a packet IO interface. >> >> Is this description enough for a new user to understand >> what will occur? >> >> >> Again, yes. >> >> >> > + * >> > + * @param[in] id ODP packet IO handle. >> >> Does not line up >> >> hdl should be the standard way for ODP to specify the handle. >> >> >> > + * @param[out] mtu Pointer to location in which to store the >> MTU value. >> > + * >> > + * @return 0 on success, -1 on error. >> > + */ >> > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); >> >> unsigned int *odp_pktio_mtu(odp_pktio_t id); >> >> @retval mtu on succes >> @retval NULL on error <list all the errors> >> >> >> No, this routine has an output parameter. This is just following standard >> ODP RC conventions for reporting success/failure as an int function return >> value. NULL can only be assigned to a pointer, not an int. Regarding the >> errors, again the ioctl will set errno so if the caller want's to know why >> it failed it can read that. It's system-dependent since each interface may >> be slightly different. >> >> >> > + >> > /** >> > * @} >> > */ >> > diff --git >> a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> > index 23633ed..ff73970 100644 >> > --- a/platform/linux-generic/include/odp_packet_io_internal.h >> > +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> > @@ -21,6 +21,8 @@ extern "C" { >> > #include <odp_spinlock.h> >> > #include <odp_packet_socket.h> >> > >> > +#include <linux/if.h> >> > + >> > /** >> > * Packet IO types >> > */ >> > @@ -38,6 +40,8 @@ struct pktio_entry { >> > odp_pktio_type_t type; /**< pktio type */ >> > pkt_sock_t pkt_sock; /**< using socket API for >> IO */ >> > pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API >> for IO */ >> > + char name[IFNAMSIZ]; /**< name of pktio provided to >> > + pktio_open() */ >> >> Does not line up. >> >> >> Doesn't checkpatch catch actual style errors? I'm assuming this is >> checkpatch clean. If it is that should be sufficient. If it's not, then it >> should be made checkpatch-clean before posting it. >> >> >> Cheers, >> Anders >> >> > }; >> > >> > typedef union { >> > diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> > index f35193f..20fe10a 100644 >> > --- a/platform/linux-generic/odp_packet_io.c >> > +++ b/platform/linux-generic/odp_packet_io.c >> > @@ -20,6 +20,7 @@ >> > #include <odp_debug.h> >> > >> > #include <string.h> >> > +#include <sys/ioctl.h> >> > >> > typedef struct { >> > pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; >> > @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, >> odp_buffer_pool_t pool) >> > return ODP_PKTIO_INVALID; >> > >> > done: >> > + strncpy(pktio_entry->s.name <http://s.name>, dev, IFNAMSIZ); >> > unlock_entry(pktio_entry); >> > return id; >> > } >> > @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, >> odp_buffer_hdr_t *buf_hdr[], int num) >> > >> > return nbr; >> > } >> > + >> > +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) >> > +{ >> > + pktio_entry_t *pktio_entry = get_entry(id); >> > + int sockfd; >> > + struct ifreq ifr; >> > + int ret; >> > + >> > + if (pktio_entry->s.pkt_sock_mmap.sockfd) >> > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; >> > + else >> > + sockfd = pktio_entry->s.pkt_sock.sockfd; >> > + >> > + strncpy(ifr.ifr_name, pktio_entry->s.name <http://s.name>, >> IFNAMSIZ); >> > + ifr.ifr_mtu = mtu; >> > + >> > + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); >> > + if (ret != 0) { >> > + ODP_ERR("ioctl SIOCSIFMTU error\n"); >> > + return -1; >> > + } >> > + >> > + return 0; >> > +} >> > + >> > +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) >> > +{ >> > + pktio_entry_t *pktio_entry = get_entry(id); >> > + int sockfd; >> > + struct ifreq ifr; >> > + int ret; >> > + >> > + if (pktio_entry->s.pkt_sock_mmap.sockfd) >> > + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; >> > + else >> > + sockfd = pktio_entry->s.pkt_sock.sockfd; >> > + >> > + strncpy(ifr.ifr_name, pktio_entry->s.name <http://s.name>, >> IFNAMSIZ); >> > + >> > + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); >> > + if (ret != 0) { >> > + ODP_ERR("ioctl SIOCGIFMTU error\n"); >> > + return -1; >> > + } >> > + >> > + *mtu = ifr.ifr_mtu; >> > + return 0; >> > +} >> > -- >> > 1.8.5.1.163.gd7aced9 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> -- >> Anders Roxell >> anders.roxell@linaro.org <mailto:anders.roxell@linaro.org> >> M: +46 709 71 42 85 <tel:%2B46%20709%2071%2042%2085> | IRC: roxell >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h index 360636d..b784705 100644 --- a/platform/linux-generic/include/api/odp_packet_io.h +++ b/platform/linux-generic/include/api/odp_packet_io.h @@ -135,6 +135,26 @@ void odp_pktio_set_input(odp_packet_t pkt, odp_pktio_t id); */ odp_pktio_t odp_pktio_get_input(odp_packet_t pkt); +/* + * Configure the MTU for a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[in] mtu The MTU to be applied. + * + * @return 0 on success, -1 on error. + */ +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu); + +/* + * Get the currently configured MTU of a packet IO interface. + * + * @param[in] id ODP packet IO handle. + * @param[out] mtu Pointer to location in which to store the MTU value. + * + * @return 0 on success, -1 on error. + */ +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu); + /** * @} */ diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 23633ed..ff73970 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -21,6 +21,8 @@ extern "C" { #include <odp_spinlock.h> #include <odp_packet_socket.h> +#include <linux/if.h> + /** * Packet IO types */ @@ -38,6 +40,8 @@ struct pktio_entry { odp_pktio_type_t type; /**< pktio type */ pkt_sock_t pkt_sock; /**< using socket API for IO */ pkt_sock_mmap_t pkt_sock_mmap; /**< using socket mmap API for IO */ + char name[IFNAMSIZ]; /**< name of pktio provided to + pktio_open() */ }; typedef union { diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index f35193f..20fe10a 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -20,6 +20,7 @@ #include <odp_debug.h> #include <string.h> +#include <sys/ioctl.h> typedef struct { pktio_entry_t entries[ODP_CONFIG_PKTIO_ENTRIES]; @@ -203,6 +204,7 @@ odp_pktio_t odp_pktio_open(const char *dev, odp_buffer_pool_t pool) return ODP_PKTIO_INVALID; done: + strncpy(pktio_entry->s.name, dev, IFNAMSIZ); unlock_entry(pktio_entry); return id; } @@ -476,3 +478,51 @@ int pktin_deq_multi(queue_entry_t *qentry, odp_buffer_hdr_t *buf_hdr[], int num) return nbr; } + +int odp_pktio_set_mtu(odp_pktio_t id, unsigned int mtu) +{ + pktio_entry_t *pktio_entry = get_entry(id); + int sockfd; + struct ifreq ifr; + int ret; + + if (pktio_entry->s.pkt_sock_mmap.sockfd) + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; + else + sockfd = pktio_entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); + ifr.ifr_mtu = mtu; + + ret = ioctl(sockfd, SIOCSIFMTU, (caddr_t)&ifr); + if (ret != 0) { + ODP_ERR("ioctl SIOCSIFMTU error\n"); + return -1; + } + + return 0; +} + +int odp_pktio_mtu(odp_pktio_t id, unsigned int *mtu) +{ + pktio_entry_t *pktio_entry = get_entry(id); + int sockfd; + struct ifreq ifr; + int ret; + + if (pktio_entry->s.pkt_sock_mmap.sockfd) + sockfd = pktio_entry->s.pkt_sock_mmap.sockfd; + else + sockfd = pktio_entry->s.pkt_sock.sockfd; + + strncpy(ifr.ifr_name, pktio_entry->s.name, IFNAMSIZ); + + ret = ioctl(sockfd, SIOCGIFMTU, &ifr); + if (ret != 0) { + ODP_ERR("ioctl SIOCGIFMTU error\n"); + return -1; + } + + *mtu = ifr.ifr_mtu; + return 0; +}
Implement pktio mtu functions: odp_pktio_mtu() to get mtu value; odp_pktio_set_mtu() to set mtu value. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- platform/linux-generic/include/api/odp_packet_io.h | 20 +++++++++ .../linux-generic/include/odp_packet_io_internal.h | 4 ++ platform/linux-generic/odp_packet_io.c | 50 ++++++++++++++++++++++ 3 files changed, 74 insertions(+)