linux-generic: pktio: print out the name of pktio used

Message ID	1446230294-10534-1-git-send-email-zoltan.kiss@linaro.org
State	Superseded
Headers	show Delivered-To: patch@linaro.org Received-SPF: pass (google.com: domain of lng-odp-bounces@lists.linaro.org designates 54.225.227.206 as permitted sender) client-ip=54.225.227.206; From: Zoltan Kiss <zoltan.kiss@linaro.org> To: lng-odp@lists.linaro.org Date: Fri, 30 Oct 2015 18:38:14 +0000 Message-Id: <1446230294-10534-1-git-send-email-zoltan.kiss@linaro.org> Subject: [lng-odp] [PATCH] linux-generic: pktio: print out the name of pktio used Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: lng-odp-bounces@lists.linaro.org Sender: "lng-odp" <lng-odp-bounces@lists.linaro.org>

Zoltan Kiss Oct. 30, 2015, 6:38 p.m. UTC

For debug purposes, otherwise it's not trivial to figure out which pktio was
successful.

Signed-off-by: Zoltan Kiss <zoltan.kiss@linaro.org>

Maxim Uvarov Nov. 2, 2015, 9:29 a.m. UTC | #1

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

Maxim Uvarov Nov. 2, 2015, 10:25 a.m. UTC | #2

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

Stuart Haslam Nov. 2, 2015, 10:54 a.m. UTC | #3

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.

Zoltan Kiss Nov. 2, 2015, 5:31 p.m. UTC | #4

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

Maxim Uvarov Nov. 2, 2015, 5:54 p.m. UTC | #5

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

Zoltan Kiss Nov. 2, 2015, 5:56 p.m. UTC | #6

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

Zoltan Kiss Nov. 2, 2015, 5:57 p.m. UTC | #7

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)

Zoltan Kiss Nov. 10, 2015, 3:54 p.m. UTC | #8

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

Stuart Haslam Nov. 10, 2015, 6:26 p.m. UTC | #9

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.

linux-generic: pktio: print out the name of pktio used

Commit Message

Comments

Patch