Message ID | 1446230294-10534-1-git-send-email-zoltan.kiss@linaro.org |
---|---|
State | Superseded |
Headers | show |
Please add that to odp_pktio_print() call in api-next. Maxim. On 10/30/2015 21:38, Zoltan Kiss wrote: > For debug purposes, otherwise it's not trivial to figure out which pktio was > successful. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h > index 4745bd5..4432cfc 100644 > --- a/platform/linux-generic/include/odp_packet_io_internal.h > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > @@ -94,6 +94,7 @@ typedef struct { > } pktio_table_t; > > typedef struct pktio_if_ops { > + const char *name; > int (*init)(void); > int (*term)(void); > int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index 1246bff..a4b1533 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char *dev, odp_pool_t pool, > > if (!ret) { > pktio_entry->s.ops = pktio_if_ops[pktio_if]; > + ODP_DBG("%s uses %s\n", > + dev, pktio_if_ops[pktio_if]->name); > break; > } > } > diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c > index 0d8dadd..8efa611 100644 > --- a/platform/linux-generic/pktio/loop.c > +++ b/platform/linux-generic/pktio/loop.c > @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t loopback_pktio_ops = { > + .name = "loopback", > .init = NULL, > .term = NULL, > .open = loopback_open, > diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c > index 794c82e..bc4ab1c 100644 > --- a/platform/linux-generic/pktio/netmap.c > +++ b/platform/linux-generic/pktio/netmap.c > @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t netmap_pktio_ops = { > + .name = "netmap", > .init = NULL, > .term = NULL, > .open = netmap_open, > diff --git a/platform/linux-generic/pktio/pcap.c b/platform/linux-generic/pktio/pcap.c > index 0817bf5..94b506d 100644 > --- a/platform/linux-generic/pktio/pcap.c > +++ b/platform/linux-generic/pktio/pcap.c > @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t pcap_pktio_ops = { > + .name = "pcap", > .open = pcapif_init, > .close = pcapif_close, > .recv = pcapif_recv_pkt, > diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c > index 7e30027..66fd9ca 100644 > --- a/platform/linux-generic/pktio/socket.c > +++ b/platform/linux-generic/pktio/socket.c > @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t sock_mmsg_pktio_ops = { > + .name = "sock_mmsg", > .init = NULL, > .term = NULL, > .open = sock_mmsg_open, > diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux-generic/pktio/socket_mmap.c > index 35d24c6..cdf221e 100644 > --- a/platform/linux-generic/pktio/socket_mmap.c > +++ b/platform/linux-generic/pktio/socket_mmap.c > @@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t sock_mmap_pktio_ops = { > + .name = "sock_mmap", > .init = NULL, > .term = NULL, > .open = sock_mmap_open, > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
On 11/02/2015 13:22, Elo, Matias (Nokia - FI/Espoo) wrote: > I added the odp_pktio_print() function while thinking just this use case. Comments below. > > -Matias > >> -----Original Message----- >> From: lng-odp [mailto:lng-odp-bounces@lists.linaro.org] On Behalf Of EXT Zoltan >> Kiss >> Sent: Friday, October 30, 2015 8:38 PM >> To: lng-odp@lists.linaro.org >> Subject: [lng-odp] [PATCH] linux-generic: pktio: print out the name of pktio used >> >> For debug purposes, otherwise it's not trivial to figure out which pktio was >> successful. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >> >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> index 4745bd5..4432cfc 100644 >> --- a/platform/linux-generic/include/odp_packet_io_internal.h >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> @@ -94,6 +94,7 @@ typedef struct { >> } pktio_table_t; >> >> typedef struct pktio_if_ops { >> + const char *name; >> int (*init)(void); >> int (*term)(void); >> int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, > This struct includes only function pointers, so it could be cleaner to use something like: > const char *(*type_string)(void); I think it's better to name it just info_str: const char *(*info_str)(void); Maxim. > >> diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux- >> generic/odp_packet_io.c >> index 1246bff..a4b1533 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char *dev, >> odp_pool_t pool, >> >> if (!ret) { >> pktio_entry->s.ops = pktio_if_ops[pktio_if]; >> + ODP_DBG("%s uses %s\n", >> + dev, pktio_if_ops[pktio_if]->name); >> break; >> } >> } > odp_pktio_print() can be used from the application. > >> diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux- >> generic/pktio/loop.c >> index 0d8dadd..8efa611 100644 >> --- a/platform/linux-generic/pktio/loop.c >> +++ b/platform/linux-generic/pktio/loop.c >> @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> >> const pktio_if_ops_t loopback_pktio_ops = { >> + .name = "loopback", >> .init = NULL, >> .term = NULL, >> .open = loopback_open, > Earlier comment about function pointers. > >> diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux- >> generic/pktio/netmap.c >> index 794c82e..bc4ab1c 100644 >> --- a/platform/linux-generic/pktio/netmap.c >> +++ b/platform/linux-generic/pktio/netmap.c >> @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> >> const pktio_if_ops_t netmap_pktio_ops = { >> + .name = "netmap", >> .init = NULL, >> .term = NULL, >> .open = netmap_open, >> diff --git a/platform/linux-generic/pktio/pcap.c b/platform/linux- >> generic/pktio/pcap.c >> index 0817bf5..94b506d 100644 >> --- a/platform/linux-generic/pktio/pcap.c >> +++ b/platform/linux-generic/pktio/pcap.c >> @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> >> const pktio_if_ops_t pcap_pktio_ops = { >> + .name = "pcap", >> .open = pcapif_init, >> .close = pcapif_close, >> .recv = pcapif_recv_pkt, >> diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux- >> generic/pktio/socket.c >> index 7e30027..66fd9ca 100644 >> --- a/platform/linux-generic/pktio/socket.c >> +++ b/platform/linux-generic/pktio/socket.c >> @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> >> const pktio_if_ops_t sock_mmsg_pktio_ops = { >> + .name = "sock_mmsg", >> .init = NULL, >> .term = NULL, >> .open = sock_mmsg_open, >> diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux- >> generic/pktio/socket_mmap.c >> index 35d24c6..cdf221e 100644 >> --- a/platform/linux-generic/pktio/socket_mmap.c >> +++ b/platform/linux-generic/pktio/socket_mmap.c >> @@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> >> const pktio_if_ops_t sock_mmap_pktio_ops = { >> + .name = "sock_mmap", >> .init = NULL, >> .term = NULL, >> .open = sock_mmap_open, >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
On Fri, Oct 30, 2015 at 06:38:14PM +0000, Zoltan Kiss wrote: > For debug purposes, otherwise it's not trivial to figure out which pktio was > successful. > > Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > > diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h > index 4745bd5..4432cfc 100644 > --- a/platform/linux-generic/include/odp_packet_io_internal.h > +++ b/platform/linux-generic/include/odp_packet_io_internal.h > @@ -94,6 +94,7 @@ typedef struct { > } pktio_table_t; > > typedef struct pktio_if_ops { > + const char *name; > int (*init)(void); > int (*term)(void); > int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, > diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c > index 1246bff..a4b1533 100644 > --- a/platform/linux-generic/odp_packet_io.c > +++ b/platform/linux-generic/odp_packet_io.c > @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char *dev, odp_pool_t pool, > > if (!ret) { > pktio_entry->s.ops = pktio_if_ops[pktio_if]; > + ODP_DBG("%s uses %s\n", > + dev, pktio_if_ops[pktio_if]->name); > break; > } > } > diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c > index 0d8dadd..8efa611 100644 > --- a/platform/linux-generic/pktio/loop.c > +++ b/platform/linux-generic/pktio/loop.c > @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t loopback_pktio_ops = { > + .name = "loopback", "loop" would be better as that's the prefix used. It would be good in future if all of these names could be used as prefixes when opening the pktio to force use of a particular pktio type. > .init = NULL, > .term = NULL, > .open = loopback_open, > diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c > index 794c82e..bc4ab1c 100644 > --- a/platform/linux-generic/pktio/netmap.c > +++ b/platform/linux-generic/pktio/netmap.c > @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t netmap_pktio_ops = { > + .name = "netmap", > .init = NULL, > .term = NULL, > .open = netmap_open, > diff --git a/platform/linux-generic/pktio/pcap.c b/platform/linux-generic/pktio/pcap.c > index 0817bf5..94b506d 100644 > --- a/platform/linux-generic/pktio/pcap.c > +++ b/platform/linux-generic/pktio/pcap.c > @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t pcap_pktio_ops = { > + .name = "pcap", > .open = pcapif_init, > .close = pcapif_close, > .recv = pcapif_recv_pkt, > diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c > index 7e30027..66fd9ca 100644 > --- a/platform/linux-generic/pktio/socket.c > +++ b/platform/linux-generic/pktio/socket.c > @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t sock_mmsg_pktio_ops = { > + .name = "sock_mmsg", > .init = NULL, > .term = NULL, > .open = sock_mmsg_open, > diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux-generic/pktio/socket_mmap.c > index 35d24c6..cdf221e 100644 > --- a/platform/linux-generic/pktio/socket_mmap.c > +++ b/platform/linux-generic/pktio/socket_mmap.c > @@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) > } > > const pktio_if_ops_t sock_mmap_pktio_ops = { > + .name = "sock_mmap", > .init = NULL, > .term = NULL, > .open = sock_mmap_open, How about "socket" and "socket_mmap"? that way the names match the source file names. -- Stuart.
Hi, Yes, it makes sense to add it there as well, but I think that ODP_DBG printout should stay as well. So you don't have to modify your app while debugging if you only want to know which pktio were used. I'm not sure which one you meant: move it there or keep it both? Zoli On 02/11/15 09:29, Maxim Uvarov wrote: > Please add that to odp_pktio_print() call in api-next. > > Maxim. > > On 10/30/2015 21:38, Zoltan Kiss wrote: >> For debug purposes, otherwise it's not trivial to figure out which >> pktio was >> successful. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >> >> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >> b/platform/linux-generic/include/odp_packet_io_internal.h >> index 4745bd5..4432cfc 100644 >> --- a/platform/linux-generic/include/odp_packet_io_internal.h >> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >> @@ -94,6 +94,7 @@ typedef struct { >> } pktio_table_t; >> typedef struct pktio_if_ops { >> + const char *name; >> int (*init)(void); >> int (*term)(void); >> int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, >> diff --git a/platform/linux-generic/odp_packet_io.c >> b/platform/linux-generic/odp_packet_io.c >> index 1246bff..a4b1533 100644 >> --- a/platform/linux-generic/odp_packet_io.c >> +++ b/platform/linux-generic/odp_packet_io.c >> @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char >> *dev, odp_pool_t pool, >> if (!ret) { >> pktio_entry->s.ops = pktio_if_ops[pktio_if]; >> + ODP_DBG("%s uses %s\n", >> + dev, pktio_if_ops[pktio_if]->name); >> break; >> } >> } >> diff --git a/platform/linux-generic/pktio/loop.c >> b/platform/linux-generic/pktio/loop.c >> index 0d8dadd..8efa611 100644 >> --- a/platform/linux-generic/pktio/loop.c >> +++ b/platform/linux-generic/pktio/loop.c >> @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> const pktio_if_ops_t loopback_pktio_ops = { >> + .name = "loopback", >> .init = NULL, >> .term = NULL, >> .open = loopback_open, >> diff --git a/platform/linux-generic/pktio/netmap.c >> b/platform/linux-generic/pktio/netmap.c >> index 794c82e..bc4ab1c 100644 >> --- a/platform/linux-generic/pktio/netmap.c >> +++ b/platform/linux-generic/pktio/netmap.c >> @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> const pktio_if_ops_t netmap_pktio_ops = { >> + .name = "netmap", >> .init = NULL, >> .term = NULL, >> .open = netmap_open, >> diff --git a/platform/linux-generic/pktio/pcap.c >> b/platform/linux-generic/pktio/pcap.c >> index 0817bf5..94b506d 100644 >> --- a/platform/linux-generic/pktio/pcap.c >> +++ b/platform/linux-generic/pktio/pcap.c >> @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> const pktio_if_ops_t pcap_pktio_ops = { >> + .name = "pcap", >> .open = pcapif_init, >> .close = pcapif_close, >> .recv = pcapif_recv_pkt, >> diff --git a/platform/linux-generic/pktio/socket.c >> b/platform/linux-generic/pktio/socket.c >> index 7e30027..66fd9ca 100644 >> --- a/platform/linux-generic/pktio/socket.c >> +++ b/platform/linux-generic/pktio/socket.c >> @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t >> *pktio_entry) >> } >> const pktio_if_ops_t sock_mmsg_pktio_ops = { >> + .name = "sock_mmsg", >> .init = NULL, >> .term = NULL, >> .open = sock_mmsg_open, >> diff --git a/platform/linux-generic/pktio/socket_mmap.c >> b/platform/linux-generic/pktio/socket_mmap.c >> index 35d24c6..cdf221e 100644 >> --- a/platform/linux-generic/pktio/socket_mmap.c >> +++ b/platform/linux-generic/pktio/socket_mmap.c >> @@ -516,6 +516,7 @@ static int >> sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) >> } >> const pktio_if_ops_t sock_mmap_pktio_ops = { >> + .name = "sock_mmap", >> .init = NULL, >> .term = NULL, >> .open = sock_mmap_open, >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
On 11/02/2015 20:31, Zoltan Kiss wrote: > Hi, > > Yes, it makes sense to add it there as well, but I think that ODP_DBG > printout should stay as well. So you don't have to modify your app > while debugging if you only want to know which pktio were used. > I'm not sure which one you meant: move it there or keep it both? keep it both. Maxim. > > Zoli > > On 02/11/15 09:29, Maxim Uvarov wrote: >> Please add that to odp_pktio_print() call in api-next. >> >> Maxim. >> >> On 10/30/2015 21:38, Zoltan Kiss wrote: >>> For debug purposes, otherwise it's not trivial to figure out which >>> pktio was >>> successful. >>> >>> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >>> >>> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >>> b/platform/linux-generic/include/odp_packet_io_internal.h >>> index 4745bd5..4432cfc 100644 >>> --- a/platform/linux-generic/include/odp_packet_io_internal.h >>> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >>> @@ -94,6 +94,7 @@ typedef struct { >>> } pktio_table_t; >>> typedef struct pktio_if_ops { >>> + const char *name; >>> int (*init)(void); >>> int (*term)(void); >>> int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, >>> diff --git a/platform/linux-generic/odp_packet_io.c >>> b/platform/linux-generic/odp_packet_io.c >>> index 1246bff..a4b1533 100644 >>> --- a/platform/linux-generic/odp_packet_io.c >>> +++ b/platform/linux-generic/odp_packet_io.c >>> @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char >>> *dev, odp_pool_t pool, >>> if (!ret) { >>> pktio_entry->s.ops = pktio_if_ops[pktio_if]; >>> + ODP_DBG("%s uses %s\n", >>> + dev, pktio_if_ops[pktio_if]->name); >>> break; >>> } >>> } >>> diff --git a/platform/linux-generic/pktio/loop.c >>> b/platform/linux-generic/pktio/loop.c >>> index 0d8dadd..8efa611 100644 >>> --- a/platform/linux-generic/pktio/loop.c >>> +++ b/platform/linux-generic/pktio/loop.c >>> @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t >>> *pktio_entry) >>> } >>> const pktio_if_ops_t loopback_pktio_ops = { >>> + .name = "loopback", >>> .init = NULL, >>> .term = NULL, >>> .open = loopback_open, >>> diff --git a/platform/linux-generic/pktio/netmap.c >>> b/platform/linux-generic/pktio/netmap.c >>> index 794c82e..bc4ab1c 100644 >>> --- a/platform/linux-generic/pktio/netmap.c >>> +++ b/platform/linux-generic/pktio/netmap.c >>> @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t >>> *pktio_entry) >>> } >>> const pktio_if_ops_t netmap_pktio_ops = { >>> + .name = "netmap", >>> .init = NULL, >>> .term = NULL, >>> .open = netmap_open, >>> diff --git a/platform/linux-generic/pktio/pcap.c >>> b/platform/linux-generic/pktio/pcap.c >>> index 0817bf5..94b506d 100644 >>> --- a/platform/linux-generic/pktio/pcap.c >>> +++ b/platform/linux-generic/pktio/pcap.c >>> @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t >>> *pktio_entry) >>> } >>> const pktio_if_ops_t pcap_pktio_ops = { >>> + .name = "pcap", >>> .open = pcapif_init, >>> .close = pcapif_close, >>> .recv = pcapif_recv_pkt, >>> diff --git a/platform/linux-generic/pktio/socket.c >>> b/platform/linux-generic/pktio/socket.c >>> index 7e30027..66fd9ca 100644 >>> --- a/platform/linux-generic/pktio/socket.c >>> +++ b/platform/linux-generic/pktio/socket.c >>> @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t >>> *pktio_entry) >>> } >>> const pktio_if_ops_t sock_mmsg_pktio_ops = { >>> + .name = "sock_mmsg", >>> .init = NULL, >>> .term = NULL, >>> .open = sock_mmsg_open, >>> diff --git a/platform/linux-generic/pktio/socket_mmap.c >>> b/platform/linux-generic/pktio/socket_mmap.c >>> index 35d24c6..cdf221e 100644 >>> --- a/platform/linux-generic/pktio/socket_mmap.c >>> +++ b/platform/linux-generic/pktio/socket_mmap.c >>> @@ -516,6 +516,7 @@ static int >>> sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) >>> } >>> const pktio_if_ops_t sock_mmap_pktio_ops = { >>> + .name = "sock_mmap", >>> .init = NULL, >>> .term = NULL, >>> .open = sock_mmap_open, >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp
Hi, On 02/11/15 10:25, Maxim Uvarov wrote: >>> diff --git a/platform/linux-generic/include/odp_packet_io_internal.h >>> b/platform/linux-generic/include/odp_packet_io_internal.h >>> index 4745bd5..4432cfc 100644 >>> --- a/platform/linux-generic/include/odp_packet_io_internal.h >>> +++ b/platform/linux-generic/include/odp_packet_io_internal.h >>> @@ -94,6 +94,7 @@ typedef struct { >>> } pktio_table_t; >>> >>> typedef struct pktio_if_ops { >>> + const char *name; >>> int (*init)(void); >>> int (*term)(void); >>> int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, >> This struct includes only function pointers, so it could be cleaner to >> use something like: >> const char *(*type_string)(void); > I think it's better to name it just info_str: > > const char *(*info_str)(void); I don't think it would be cleaner: I just want a simple string which contains the name of the instance, not a function pointer which returns a string. And I don't see any reason why the fact there are only function pointers in the struct at the moment means we shouldn't change that. I saw a lot of other places in different project where they used a const char array to store some kind of name for the instance of that struct. See 'git grep "const char.*name;"' in ODP as well. Zoli
On 02/11/15 10:54, Stuart Haslam wrote: >> diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c >> >index 0d8dadd..8efa611 100644 >> >--- a/platform/linux-generic/pktio/loop.c >> >+++ b/platform/linux-generic/pktio/loop.c >> >@@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t *pktio_entry) >> > } >> > >> > const pktio_if_ops_t loopback_pktio_ops = { >> >+ .name = "loopback", > "loop" would be better as that's the prefix used. Ok. (also the other name change suggestion)
Hi, On 02/11/15 10:54, Stuart Haslam wrote: > On Fri, Oct 30, 2015 at 06:38:14PM +0000, Zoltan Kiss wrote: >> For debug purposes, otherwise it's not trivial to figure out which pktio was >> successful. >> >> Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> >> >> diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c >> index 7e30027..66fd9ca 100644 >> --- a/platform/linux-generic/pktio/socket.c >> +++ b/platform/linux-generic/pktio/socket.c >> @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t *pktio_entry) >> } >> >> const pktio_if_ops_t sock_mmsg_pktio_ops = { >> + .name = "sock_mmsg", >> .init = NULL, >> .term = NULL, >> .open = sock_mmsg_open, >> diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux-generic/pktio/socket_mmap.c >> index 35d24c6..cdf221e 100644 >> --- a/platform/linux-generic/pktio/socket_mmap.c >> +++ b/platform/linux-generic/pktio/socket_mmap.c >> @@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) >> } >> >> const pktio_if_ops_t sock_mmap_pktio_ops = { >> + .name = "sock_mmap", >> .init = NULL, >> .term = NULL, >> .open = sock_mmap_open, > > How about "socket" and "socket_mmap"? that way the names match the > source file names. Now that I wanted to update the patch, I found that although the file names use "socket", all the function names use "sock". Which one should we align to? Zoli
On Tue, Nov 10, 2015 at 03:54:31PM +0000, Zoltan Kiss wrote: > Hi, > > On 02/11/15 10:54, Stuart Haslam wrote: > >On Fri, Oct 30, 2015 at 06:38:14PM +0000, Zoltan Kiss wrote: > >>For debug purposes, otherwise it's not trivial to figure out which pktio was > >>successful. > >> > >>Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org> > >> > > >>diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c > >>index 7e30027..66fd9ca 100644 > >>--- a/platform/linux-generic/pktio/socket.c > >>+++ b/platform/linux-generic/pktio/socket.c > >>@@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t *pktio_entry) > >> } > >> > >> const pktio_if_ops_t sock_mmsg_pktio_ops = { > >>+ .name = "sock_mmsg", > >> .init = NULL, > >> .term = NULL, > >> .open = sock_mmsg_open, > >>diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux-generic/pktio/socket_mmap.c > >>index 35d24c6..cdf221e 100644 > >>--- a/platform/linux-generic/pktio/socket_mmap.c > >>+++ b/platform/linux-generic/pktio/socket_mmap.c > >>@@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) > >> } > >> > >> const pktio_if_ops_t sock_mmap_pktio_ops = { > >>+ .name = "sock_mmap", > >> .init = NULL, > >> .term = NULL, > >> .open = sock_mmap_open, > > > >How about "socket" and "socket_mmap"? that way the names match the > >source file names. > > Now that I wanted to update the patch, I found that although the > file names use "socket", all the function names use "sock". Which > one should we align to? > > Zoli I was thinking of the filename as being the externally visible module name whereas the function names are internal. I don't mind much either way though, so long as the "_mmsg" bit is removed.
diff --git a/platform/linux-generic/include/odp_packet_io_internal.h b/platform/linux-generic/include/odp_packet_io_internal.h index 4745bd5..4432cfc 100644 --- a/platform/linux-generic/include/odp_packet_io_internal.h +++ b/platform/linux-generic/include/odp_packet_io_internal.h @@ -94,6 +94,7 @@ typedef struct { } pktio_table_t; typedef struct pktio_if_ops { + const char *name; int (*init)(void); int (*term)(void); int (*open)(odp_pktio_t pktio, pktio_entry_t *pktio_entry, diff --git a/platform/linux-generic/odp_packet_io.c b/platform/linux-generic/odp_packet_io.c index 1246bff..a4b1533 100644 --- a/platform/linux-generic/odp_packet_io.c +++ b/platform/linux-generic/odp_packet_io.c @@ -229,6 +229,8 @@ static odp_pktio_t setup_pktio_entry(const char *dev, odp_pool_t pool, if (!ret) { pktio_entry->s.ops = pktio_if_ops[pktio_if]; + ODP_DBG("%s uses %s\n", + dev, pktio_if_ops[pktio_if]->name); break; } } diff --git a/platform/linux-generic/pktio/loop.c b/platform/linux-generic/pktio/loop.c index 0d8dadd..8efa611 100644 --- a/platform/linux-generic/pktio/loop.c +++ b/platform/linux-generic/pktio/loop.c @@ -109,6 +109,7 @@ static int loopback_promisc_mode_get(pktio_entry_t *pktio_entry) } const pktio_if_ops_t loopback_pktio_ops = { + .name = "loopback", .init = NULL, .term = NULL, .open = loopback_open, diff --git a/platform/linux-generic/pktio/netmap.c b/platform/linux-generic/pktio/netmap.c index 794c82e..bc4ab1c 100644 --- a/platform/linux-generic/pktio/netmap.c +++ b/platform/linux-generic/pktio/netmap.c @@ -307,6 +307,7 @@ static int netmap_promisc_mode_get(pktio_entry_t *pktio_entry) } const pktio_if_ops_t netmap_pktio_ops = { + .name = "netmap", .init = NULL, .term = NULL, .open = netmap_open, diff --git a/platform/linux-generic/pktio/pcap.c b/platform/linux-generic/pktio/pcap.c index 0817bf5..94b506d 100644 --- a/platform/linux-generic/pktio/pcap.c +++ b/platform/linux-generic/pktio/pcap.c @@ -370,6 +370,7 @@ static int pcapif_promisc_mode_get(pktio_entry_t *pktio_entry) } const pktio_if_ops_t pcap_pktio_ops = { + .name = "pcap", .open = pcapif_init, .close = pcapif_close, .recv = pcapif_recv_pkt, diff --git a/platform/linux-generic/pktio/socket.c b/platform/linux-generic/pktio/socket.c index 7e30027..66fd9ca 100644 --- a/platform/linux-generic/pktio/socket.c +++ b/platform/linux-generic/pktio/socket.c @@ -463,6 +463,7 @@ static int sock_promisc_mode_get(pktio_entry_t *pktio_entry) } const pktio_if_ops_t sock_mmsg_pktio_ops = { + .name = "sock_mmsg", .init = NULL, .term = NULL, .open = sock_mmsg_open, diff --git a/platform/linux-generic/pktio/socket_mmap.c b/platform/linux-generic/pktio/socket_mmap.c index 35d24c6..cdf221e 100644 --- a/platform/linux-generic/pktio/socket_mmap.c +++ b/platform/linux-generic/pktio/socket_mmap.c @@ -516,6 +516,7 @@ static int sock_mmap_promisc_mode_get(pktio_entry_t *pktio_entry) } const pktio_if_ops_t sock_mmap_pktio_ops = { + .name = "sock_mmap", .init = NULL, .term = NULL, .open = sock_mmap_open,
For debug purposes, otherwise it's not trivial to figure out which pktio was successful. Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>