[ovs-dev,2/2] netdev-dpdk: Add comment about variables naming convention.

Message ID 1510060648-30484-3-git-send-email-i.maximets@samsung.com
State Superseded
Delegated to: Ian Stokes
Headers show
Series
  • netdev-dpdk: Fix and document variables naming.
Related show

Commit Message

Ilya Maximets Nov. 7, 2017, 1:17 p.m.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/netdev-dpdk.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Stokes, Ian Nov. 22, 2017, 5:28 p.m. | #1
> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Ilya Maximets
> Sent: Tuesday, November 7, 2017 1:17 PM
> To: ovs-dev@openvswitch.org
> Cc: Ilya Maximets <i.maximets@samsung.com>; Heetae Ahn
> <heetae82.ahn@samsung.com>
> Subject: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add comment about variables
> naming convention.
> 

Hi Ilya,

This looks good and will be useful in future as a reference.

Minor nit would be I'd like to see the commit message expanded upon, maybe mentioning which variables are documented etc. to make the commit message more robust. Will look to add this to the DPDK merge branch then.

Thanks
Ian

> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/netdev-dpdk.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 872b133..9423109
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -330,6 +330,23 @@ enum dpdk_hw_ol_features {
>      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,  };
> 
> +/*
> + * In order to avoid confusion in variables names, following naming
> +convention
> + * should be used, if possible:
> + *
> + *     'struct netdev'          : 'netdev'
> + *     'struct netdev_dpdk'     : 'dev'
> + *     'struct netdev_rxq'      : 'rxq'
> + *     'struct netdev_rxq_dpdk' : 'rx'
> + *
> + * Example:
> + *     struct netdev *netdev = netdev_from_name(name);
> + *     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> + *
> + *  Also, 'netdev' should be used instead of 'dev->up', where 'netdev'
> +was
> + *  already defined.
> + */
> +
>  struct netdev_dpdk {
>      PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
>          dpdk_port_t port_id;
> --
> 2.7.4
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ilya Maximets Nov. 23, 2017, 1:50 p.m. | #2
On 22.11.2017 20:28, Stokes, Ian wrote:
> 
> 
>> -----Original Message-----
>> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
>> bounces@openvswitch.org] On Behalf Of Ilya Maximets
>> Sent: Tuesday, November 7, 2017 1:17 PM
>> To: ovs-dev@openvswitch.org
>> Cc: Ilya Maximets <i.maximets@samsung.com>; Heetae Ahn
>> <heetae82.ahn@samsung.com>
>> Subject: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add comment about variables
>> naming convention.
>>
> 
> Hi Ilya,
> 
> This looks good and will be useful in future as a reference.
> 
> Minor nit would be I'd like to see the commit message expanded upon, maybe mentioning which variables are documented etc. to make the commit message more robust. Will look to add this to the DPDK merge branch then.

OK. I've sent v2 with updated commit-message.

Best regards, Ilya Maximets.

> 
> Thanks
> Ian
> 
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  lib/netdev-dpdk.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 872b133..9423109
>> 100644
>> --- a/lib/netdev-dpdk.c
>> +++ b/lib/netdev-dpdk.c
>> @@ -330,6 +330,23 @@ enum dpdk_hw_ol_features {
>>      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,  };
>>
>> +/*
>> + * In order to avoid confusion in variables names, following naming
>> +convention
>> + * should be used, if possible:
>> + *
>> + *     'struct netdev'          : 'netdev'
>> + *     'struct netdev_dpdk'     : 'dev'
>> + *     'struct netdev_rxq'      : 'rxq'
>> + *     'struct netdev_rxq_dpdk' : 'rx'
>> + *
>> + * Example:
>> + *     struct netdev *netdev = netdev_from_name(name);
>> + *     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> + *
>> + *  Also, 'netdev' should be used instead of 'dev->up', where 'netdev'
>> +was
>> + *  already defined.
>> + */
>> +
>>  struct netdev_dpdk {
>>      PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
>>          dpdk_port_t port_id;
>> --
>> 2.7.4
>>
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> 
>
Stokes, Ian Nov. 23, 2017, 2:37 p.m. | #3
> >
> >
> >> -----Original Message-----
> >> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> >> bounces@openvswitch.org] On Behalf Of Ilya Maximets
> >> Sent: Tuesday, November 7, 2017 1:17 PM
> >> To: ovs-dev@openvswitch.org
> >> Cc: Ilya Maximets <i.maximets@samsung.com>; Heetae Ahn
> >> <heetae82.ahn@samsung.com>
> >> Subject: [ovs-dev] [PATCH 2/2] netdev-dpdk: Add comment about
> >> variables naming convention.
> >>
> >
> > Hi Ilya,
> >
> > This looks good and will be useful in future as a reference.
> >
> > Minor nit would be I'd like to see the commit message expanded upon,
> maybe mentioning which variables are documented etc. to make the commit
> message more robust. Will look to add this to the DPDK merge branch then.
> 
> OK. I've sent v2 with updated commit-message.
> 
> Best regards, Ilya Maximets.

Thanks Ilya, I'll roll these into the DPDK merge branch.

Cheers Ian

> 
> >
> > Thanks
> > Ian
> >
> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> >> ---
> >>  lib/netdev-dpdk.c | 17 +++++++++++++++++
> >>  1 file changed, 17 insertions(+)
> >>
> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >> 872b133..9423109
> >> 100644
> >> --- a/lib/netdev-dpdk.c
> >> +++ b/lib/netdev-dpdk.c
> >> @@ -330,6 +330,23 @@ enum dpdk_hw_ol_features {
> >>      NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,  };
> >>
> >> +/*
> >> + * In order to avoid confusion in variables names, following naming
> >> +convention
> >> + * should be used, if possible:
> >> + *
> >> + *     'struct netdev'          : 'netdev'
> >> + *     'struct netdev_dpdk'     : 'dev'
> >> + *     'struct netdev_rxq'      : 'rxq'
> >> + *     'struct netdev_rxq_dpdk' : 'rx'
> >> + *
> >> + * Example:
> >> + *     struct netdev *netdev = netdev_from_name(name);
> >> + *     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> >> + *
> >> + *  Also, 'netdev' should be used instead of 'dev->up', where 'netdev'
> >> +was
> >> + *  already defined.
> >> + */
> >> +
> >>  struct netdev_dpdk {
> >>      PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
> >>          dpdk_port_t port_id;
> >> --
> >> 2.7.4
> >>
> >> _______________________________________________
> >> dev mailing list
> >> dev@openvswitch.org
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> >
> >

Patch

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 872b133..9423109 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -330,6 +330,23 @@  enum dpdk_hw_ol_features {
     NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
 };
 
+/*
+ * In order to avoid confusion in variables names, following naming convention
+ * should be used, if possible:
+ *
+ *     'struct netdev'          : 'netdev'
+ *     'struct netdev_dpdk'     : 'dev'
+ *     'struct netdev_rxq'      : 'rxq'
+ *     'struct netdev_rxq_dpdk' : 'rx'
+ *
+ * Example:
+ *     struct netdev *netdev = netdev_from_name(name);
+ *     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+ *
+ *  Also, 'netdev' should be used instead of 'dev->up', where 'netdev' was
+ *  already defined.
+ */
+
 struct netdev_dpdk {
     PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
         dpdk_port_t port_id;