| Message ID | 1446060551-21029-4-git-send-email-stuart.haslam@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
If you add prefix, then getenv() has to be removed. And previous patch looks like obsolete. Maxim. On 10/28/2015 22:29, Stuart Haslam wrote: > The netmap API requires interface names to be prefixed with "netmap:", > the pktio code will do that automatically, e.g. odp_pktio_open("eth0") > becomes nm_open("netmap:eth0"). But it will append the netmap: even if > the original name already has that prefix, so odp_pktio_open("netmap:eth0") > fails. > > It's useful to be able to explicitly use the netmap interface, especially > when testing, if you don't want fallback to other pktio types. > > Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> > --- > platform/linux-generic/include/odp_packet_netmap.h | 2 ++ > platform/linux-generic/pktio/netmap.c | 26 +++++++++++++++------- > 2 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/platform/linux-generic/include/odp_packet_netmap.h b/platform/linux-generic/include/odp_packet_netmap.h > index 0577dfe..7caa92f 100644 > --- a/platform/linux-generic/include/odp_packet_netmap.h > +++ b/platform/linux-generic/include/odp_packet_netmap.h > @@ -10,6 +10,7 @@ > #include <odp/pool.h> > > #include <linux/if_ether.h> > +#include <net/if.h> > > /** Packet socket using netmap mmaped rings for both Rx and Tx */ > typedef struct { > @@ -20,6 +21,7 @@ typedef struct { > uint32_t if_flags; /**< interface flags */ > int sockfd; /**< control socket */ > unsigned char if_mac[ETH_ALEN]; /**< eth mac address */ > + char ifname[IF_NAMESIZE]; /**< interface name to be used in ioctl */ > } pkt_netmap_t; > > #endif > diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c > index 67ff95e..5378586 100644 > --- a/platform/linux-generic/pktio/netmap.c > +++ b/platform/linux-generic/pktio/netmap.c > @@ -47,8 +47,7 @@ static int netmap_do_ioctl(pktio_entry_t *pktio_entry, unsigned long cmd, > int fd = pkt_nm->sockfd; > > memset(&ifr, 0, sizeof(ifr)); > - snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", > - pktio_entry->s.name); > + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", pkt_nm->ifname); > > switch (cmd) { > case SIOCSIFFLAGS: > @@ -107,7 +106,8 @@ static int netmap_close(pktio_entry_t *pktio_entry) > static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, > const char *netdev, odp_pool_t pool) > { > - char ifname[IFNAMSIZ + 7]; /* netmap:<ifname> */ > + char nmname[IF_NAMESIZE + 7]; /* netmap:<ifname> */ > + const char *prefix; > int err; > int sockfd; > int i; > @@ -129,20 +129,30 @@ static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, > odp_buffer_pool_headroom(pool) - > odp_buffer_pool_tailroom(pool); > > + /* allow interface to be opened with or without the netmap: prefix */ > snprintf(pktio_entry->s.name, sizeof(pktio_entry->s.name), "%s", > netdev); > - snprintf(ifname, sizeof(ifname), "netmap:%s", netdev); > + > + if (strncmp(netdev, "netmap:", 7) == 0) { > + netdev += 7; > + prefix = ""; > + } else { > + prefix = "netmap:"; > + } > + > + snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev); > + snprintf(nmname, sizeof(nmname), "%s%s", prefix, pktio_entry->s.name); > > if (mmap_desc.mem == NULL) > - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL, > + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL, > NULL); > else > - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL | > + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL | > NM_OPEN_NO_MMAP, &mmap_desc); > - pkt_nm->tx_desc = nm_open(ifname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); > + pkt_nm->tx_desc = nm_open(nmname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); > > if (pkt_nm->rx_desc == NULL || pkt_nm->tx_desc == NULL) { > - ODP_ERR("nm_open(%s) failed\n", ifname); > + ODP_ERR("nm_open(%s) failed\n", nmname); > goto error; > } >
On Thu, Oct 29, 2015 at 02:49:31PM +0300, Maxim Uvarov wrote: > If you add prefix, then getenv() has to be removed. And previous > patch looks like obsolete. > > Maxim. The prefix forces use of a particular pktio type whereas the getenv() stuff disables a single type, they have different purposes (for now). The netmap: prefix is optional, so you can still open with "eth0" and let the implementation decide the best type available. I would like to get rid of all of the getenv() stuff but to do that we first need to add prefixes for the other pktio types (like "sock:", "sockmmap:"). -- Stuart. > > On 10/28/2015 22:29, Stuart Haslam wrote: > >The netmap API requires interface names to be prefixed with "netmap:", > >the pktio code will do that automatically, e.g. odp_pktio_open("eth0") > >becomes nm_open("netmap:eth0"). But it will append the netmap: even if > >the original name already has that prefix, so odp_pktio_open("netmap:eth0") > >fails. > > > >It's useful to be able to explicitly use the netmap interface, especially > >when testing, if you don't want fallback to other pktio types. > > > >Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> > >--- > > platform/linux-generic/include/odp_packet_netmap.h | 2 ++ > > platform/linux-generic/pktio/netmap.c | 26 +++++++++++++++------- > > 2 files changed, 20 insertions(+), 8 deletions(-) > > > >diff --git a/platform/linux-generic/include/odp_packet_netmap.h b/platform/linux-generic/include/odp_packet_netmap.h > >index 0577dfe..7caa92f 100644 > >--- a/platform/linux-generic/include/odp_packet_netmap.h > >+++ b/platform/linux-generic/include/odp_packet_netmap.h > >@@ -10,6 +10,7 @@ > > #include <odp/pool.h> > > #include <linux/if_ether.h> > >+#include <net/if.h> > > /** Packet socket using netmap mmaped rings for both Rx and Tx */ > > typedef struct { > >@@ -20,6 +21,7 @@ typedef struct { > > uint32_t if_flags; /**< interface flags */ > > int sockfd; /**< control socket */ > > unsigned char if_mac[ETH_ALEN]; /**< eth mac address */ > >+ char ifname[IF_NAMESIZE]; /**< interface name to be used in ioctl */ > > } pkt_netmap_t; > > #endif > >diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c > >index 67ff95e..5378586 100644 > >--- a/platform/linux-generic/pktio/netmap.c > >+++ b/platform/linux-generic/pktio/netmap.c > >@@ -47,8 +47,7 @@ static int netmap_do_ioctl(pktio_entry_t *pktio_entry, unsigned long cmd, > > int fd = pkt_nm->sockfd; > > memset(&ifr, 0, sizeof(ifr)); > >- snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", > >- pktio_entry->s.name); > >+ snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", pkt_nm->ifname); > > switch (cmd) { > > case SIOCSIFFLAGS: > >@@ -107,7 +106,8 @@ static int netmap_close(pktio_entry_t *pktio_entry) > > static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, > > const char *netdev, odp_pool_t pool) > > { > >- char ifname[IFNAMSIZ + 7]; /* netmap:<ifname> */ > >+ char nmname[IF_NAMESIZE + 7]; /* netmap:<ifname> */ > >+ const char *prefix; > > int err; > > int sockfd; > > int i; > >@@ -129,20 +129,30 @@ static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, > > odp_buffer_pool_headroom(pool) - > > odp_buffer_pool_tailroom(pool); > >+ /* allow interface to be opened with or without the netmap: prefix */ > > snprintf(pktio_entry->s.name, sizeof(pktio_entry->s.name), "%s", > > netdev); > >- snprintf(ifname, sizeof(ifname), "netmap:%s", netdev); > >+ > >+ if (strncmp(netdev, "netmap:", 7) == 0) { > >+ netdev += 7; > >+ prefix = ""; > >+ } else { > >+ prefix = "netmap:"; > >+ } > >+ > >+ snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev); > >+ snprintf(nmname, sizeof(nmname), "%s%s", prefix, pktio_entry->s.name); > > if (mmap_desc.mem == NULL) > >- pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL, > >+ pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL, > > NULL); > > else > >- pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL | > >+ pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL | > > NM_OPEN_NO_MMAP, &mmap_desc); > >- pkt_nm->tx_desc = nm_open(ifname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); > >+ pkt_nm->tx_desc = nm_open(nmname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); > > if (pkt_nm->rx_desc == NULL || pkt_nm->tx_desc == NULL) { > >- ODP_ERR("nm_open(%s) failed\n", ifname); > >+ ODP_ERR("nm_open(%s) failed\n", nmname); > > goto error; > > } > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
On 10/30/2015 13:03, Stuart Haslam wrote: > On Thu, Oct 29, 2015 at 02:49:31PM +0300, Maxim Uvarov wrote: >> If you add prefix, then getenv() has to be removed. And previous >> patch looks like obsolete. >> >> Maxim. > The prefix forces use of a particular pktio type whereas the getenv() > stuff disables a single type, they have different purposes (for now). > > The netmap: prefix is optional, so you can still open with "eth0" and > let the implementation decide the best type available. I would like to > get rid of all of the getenv() stuff but to do that we first need to add > prefixes for the other pktio types (like "sock:", "sockmmap:"). > > -- > Stuart. I also vote to get rid of that and use prefixes. I previously thought that you don't want app to use netmap until you explicitly say about that with prefix. But if you want to allow it I don't worry. Maxim. > >> On 10/28/2015 22:29, Stuart Haslam wrote: >>> The netmap API requires interface names to be prefixed with "netmap:", >>> the pktio code will do that automatically, e.g. odp_pktio_open("eth0") >>> becomes nm_open("netmap:eth0"). But it will append the netmap: even if >>> the original name already has that prefix, so odp_pktio_open("netmap:eth0") >>> fails. >>> >>> It's useful to be able to explicitly use the netmap interface, especially >>> when testing, if you don't want fallback to other pktio types. >>> >>> Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> >>> --- >>> platform/linux-generic/include/odp_packet_netmap.h | 2 ++ >>> platform/linux-generic/pktio/netmap.c | 26 +++++++++++++++------- >>> 2 files changed, 20 insertions(+), 8 deletions(-) >>> >>> diff --git a/platform/linux-generic/include/odp_packet_netmap.h b/platform/linux-generic/include/odp_packet_netmap.h >>> index 0577dfe..7caa92f 100644 >>> --- a/platform/linux-generic/include/odp_packet_netmap.h >>> +++ b/platform/linux-generic/include/odp_packet_netmap.h >>> @@ -10,6 +10,7 @@ >>> #include <odp/pool.h> >>> #include <linux/if_ether.h> >>> +#include <net/if.h> >>> /** Packet socket using netmap mmaped rings for both Rx and Tx */ >>> typedef struct { >>> @@ -20,6 +21,7 @@ typedef struct { >>> uint32_t if_flags; /**< interface flags */ >>> int sockfd; /**< control socket */ >>> unsigned char if_mac[ETH_ALEN]; /**< eth mac address */ >>> + char ifname[IF_NAMESIZE]; /**< interface name to be used in ioctl */ >>> } pkt_netmap_t; >>> #endif >>> diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c >>> index 67ff95e..5378586 100644 >>> --- a/platform/linux-generic/pktio/netmap.c >>> +++ b/platform/linux-generic/pktio/netmap.c >>> @@ -47,8 +47,7 @@ static int netmap_do_ioctl(pktio_entry_t *pktio_entry, unsigned long cmd, >>> int fd = pkt_nm->sockfd; >>> memset(&ifr, 0, sizeof(ifr)); >>> - snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", >>> - pktio_entry->s.name); >>> + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", pkt_nm->ifname); >>> switch (cmd) { >>> case SIOCSIFFLAGS: >>> @@ -107,7 +106,8 @@ static int netmap_close(pktio_entry_t *pktio_entry) >>> static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, >>> const char *netdev, odp_pool_t pool) >>> { >>> - char ifname[IFNAMSIZ + 7]; /* netmap:<ifname> */ >>> + char nmname[IF_NAMESIZE + 7]; /* netmap:<ifname> */ >>> + const char *prefix; >>> int err; >>> int sockfd; >>> int i; >>> @@ -129,20 +129,30 @@ static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, >>> odp_buffer_pool_headroom(pool) - >>> odp_buffer_pool_tailroom(pool); >>> + /* allow interface to be opened with or without the netmap: prefix */ >>> snprintf(pktio_entry->s.name, sizeof(pktio_entry->s.name), "%s", >>> netdev); >>> - snprintf(ifname, sizeof(ifname), "netmap:%s", netdev); >>> + >>> + if (strncmp(netdev, "netmap:", 7) == 0) { >>> + netdev += 7; >>> + prefix = ""; >>> + } else { >>> + prefix = "netmap:"; >>> + } >>> + >>> + snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev); >>> + snprintf(nmname, sizeof(nmname), "%s%s", prefix, pktio_entry->s.name); >>> if (mmap_desc.mem == NULL) >>> - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL, >>> + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL, >>> NULL); >>> else >>> - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL | >>> + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL | >>> NM_OPEN_NO_MMAP, &mmap_desc); >>> - pkt_nm->tx_desc = nm_open(ifname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); >>> + pkt_nm->tx_desc = nm_open(nmname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); >>> if (pkt_nm->rx_desc == NULL || pkt_nm->tx_desc == NULL) { >>> - ODP_ERR("nm_open(%s) failed\n", ifname); >>> + ODP_ERR("nm_open(%s) failed\n", nmname); >>> goto error; >>> } >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/platform/linux-generic/include/odp_packet_netmap.h b/platform/linux-generic/include/odp_packet_netmap.h index 0577dfe..7caa92f 100644 --- a/platform/linux-generic/include/odp_packet_netmap.h +++ b/platform/linux-generic/include/odp_packet_netmap.h @@ -10,6 +10,7 @@ #include <odp/pool.h> #include <linux/if_ether.h> +#include <net/if.h> /** Packet socket using netmap mmaped rings for both Rx and Tx */ typedef struct { @@ -20,6 +21,7 @@ typedef struct { uint32_t if_flags; /**< interface flags */ int sockfd; /**< control socket */ unsigned char if_mac[ETH_ALEN]; /**< eth mac address */ + char ifname[IF_NAMESIZE]; /**< interface name to be used in ioctl */ } pkt_netmap_t; #endif diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c index 67ff95e..5378586 100644 --- a/platform/linux-generic/pktio/netmap.c +++ b/platform/linux-generic/pktio/netmap.c @@ -47,8 +47,7 @@ static int netmap_do_ioctl(pktio_entry_t *pktio_entry, unsigned long cmd, int fd = pkt_nm->sockfd; memset(&ifr, 0, sizeof(ifr)); - snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", - pktio_entry->s.name); + snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", pkt_nm->ifname); switch (cmd) { case SIOCSIFFLAGS: @@ -107,7 +106,8 @@ static int netmap_close(pktio_entry_t *pktio_entry) static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, const char *netdev, odp_pool_t pool) { - char ifname[IFNAMSIZ + 7]; /* netmap:<ifname> */ + char nmname[IF_NAMESIZE + 7]; /* netmap:<ifname> */ + const char *prefix; int err; int sockfd; int i; @@ -129,20 +129,30 @@ static int netmap_open(odp_pktio_t id ODP_UNUSED, pktio_entry_t *pktio_entry, odp_buffer_pool_headroom(pool) - odp_buffer_pool_tailroom(pool); + /* allow interface to be opened with or without the netmap: prefix */ snprintf(pktio_entry->s.name, sizeof(pktio_entry->s.name), "%s", netdev); - snprintf(ifname, sizeof(ifname), "netmap:%s", netdev); + + if (strncmp(netdev, "netmap:", 7) == 0) { + netdev += 7; + prefix = ""; + } else { + prefix = "netmap:"; + } + + snprintf(pkt_nm->ifname, sizeof(pkt_nm->ifname), "%s", netdev); + snprintf(nmname, sizeof(nmname), "%s%s", prefix, pktio_entry->s.name); if (mmap_desc.mem == NULL) - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL, + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL, NULL); else - pkt_nm->rx_desc = nm_open(ifname, NULL, NETMAP_NO_TX_POLL | + pkt_nm->rx_desc = nm_open(nmname, NULL, NETMAP_NO_TX_POLL | NM_OPEN_NO_MMAP, &mmap_desc); - pkt_nm->tx_desc = nm_open(ifname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); + pkt_nm->tx_desc = nm_open(nmname, NULL, NM_OPEN_NO_MMAP, &mmap_desc); if (pkt_nm->rx_desc == NULL || pkt_nm->tx_desc == NULL) { - ODP_ERR("nm_open(%s) failed\n", ifname); + ODP_ERR("nm_open(%s) failed\n", nmname); goto error; }
The netmap API requires interface names to be prefixed with "netmap:", the pktio code will do that automatically, e.g. odp_pktio_open("eth0") becomes nm_open("netmap:eth0"). But it will append the netmap: even if the original name already has that prefix, so odp_pktio_open("netmap:eth0") fails. It's useful to be able to explicitly use the netmap interface, especially when testing, if you don't want fallback to other pktio types. Signed-off-by: Stuart Haslam <stuart.haslam@linaro.org> --- platform/linux-generic/include/odp_packet_netmap.h | 2 ++ platform/linux-generic/pktio/netmap.c | 26 +++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-)