[PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

Back in:

commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date:   Fri Dec 5 15:19:07 2014 +0100

    cmdline: fix overflow on bsd

The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
patch makes the needed correction to avoid a build break

Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
---
 lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
index 6555ec5..6fa6758 100644
--- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
+++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
@@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
 		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
 				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
 			if (cmdline_parse_etheraddr(NULL,
-							pair[1],
-							&dict->addr) < 0) {
+						    pair[1],
+						    &dict->addr,
+						    sizeof(struct ether_addr)) < 0) {
 				RTE_LOG(ERR, PMD,
 					"Invalid %s device ether address\n",
 					name);
-- 
1.9.3

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Thomas Monjalon]

Hi Neil,

2014-12-17 12:03, Neil Horman:
> Back in:
> 
> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date:   Fri Dec 5 15:19:07 2014 +0100
> 
>     cmdline: fix overflow on bsd
> 
> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> patch makes the needed correction to avoid a build break
> 
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

What is the meaning of CC here?

> --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
>  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
>  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
>  			if (cmdline_parse_etheraddr(NULL,
> -							pair[1],
> -							&dict->addr) < 0) {
> +						    pair[1],
> +						    &dict->addr,
> +						    sizeof(struct ether_addr)) < 0) {

Why not sizeof(dict->addr)?

-- 
Thomas

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Wed, Dec 17, 2014 at 11:20:26PM +0100, Thomas Monjalon wrote:
> Hi Neil,
> 
> 2014-12-17 12:03, Neil Horman:
> > Back in:
> > 
> > commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> > Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Date:   Fri Dec 5 15:19:07 2014 +0100
> > 
> >     cmdline: fix overflow on bsd
> > 
> > The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> > patch makes the needed correction to avoid a build break
> > 
> > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> What is the meaning of CC here?
> 
CC is a tag that git send-email understands.  As it implies it cc's the post to
the indicated email, and records that fact in the body of the commit.

> > --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> > +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> > @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
> >  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
> >  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
> >  			if (cmdline_parse_etheraddr(NULL,
> > -							pair[1],
> > -							&dict->addr) < 0) {
> > +						    pair[1],
> > +						    &dict->addr,
> > +						    sizeof(struct ether_addr)) < 0) {
> 
> Why not sizeof(dict->addr)?
> 
Because addr is a struct ether_addr, and I always get confused when doing sizeof
on pointer variables, so I find it more clear to specify the type exactly.  I'm
not bound to it though so if you like I can change it, though given its release
day, I figure you want to fix this build break asap.
Neil

> -- 
> Thomas
> 

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Thomas Monjalon]

2014-12-18 06:25, Neil Horman:
> On Wed, Dec 17, 2014 at 11:20:26PM +0100, Thomas Monjalon wrote:
> > 2014-12-17 12:03, Neil Horman:
> > > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > > CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> > 
> > What is the meaning of CC here?
> > 
> CC is a tag that git send-email understands.  As it implies it cc's the post to
> the indicated email, and records that fact in the body of the commit.

OK but why CC me when sending this patch?

-- 
Thomas

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Thu, Dec 18, 2014 at 01:21:31PM +0100, Thomas Monjalon wrote:
> 2014-12-18 06:25, Neil Horman:
> > On Wed, Dec 17, 2014 at 11:20:26PM +0100, Thomas Monjalon wrote:
> > > 2014-12-17 12:03, Neil Horman:
> > > > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > > > CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> > > 
> > > What is the meaning of CC here?
> > > 
> > CC is a tag that git send-email understands.  As it implies it cc's the post to
> > the indicated email, and records that fact in the body of the commit.
> 
> OK but why CC me when sending this patch?
Typically a tree maintainer is CC'ed when submitting a patch to a list. I've
done it on most, if not all of my previous patches (and where I didn't, I should
have).  Especially given that we're this late in the release cycle, I want to be
sure you're attention is called to a build break

Neil

> 
> -- 
> Thomas
> 

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Qiu, Michael]

On 2014/12/18 19:26, Neil Horman wrote:
> On Wed, Dec 17, 2014 at 11:20:26PM +0100, Thomas Monjalon wrote:
>> Hi Neil,
>>
>> 2014-12-17 12:03, Neil Horman:
>>> Back in:
>>>
>>> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
>>> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>> Date:   Fri Dec 5 15:19:07 2014 +0100
>>>
>>>     cmdline: fix overflow on bsd
>>>
>>> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
>>> patch makes the needed correction to avoid a build break
>>>
>>> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
>>> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
>> What is the meaning of CC here?
>>
> CC is a tag that git send-email understands.  As it implies it cc's the post to
> the indicated email, and records that fact in the body of the commit.

But if you use --cc in git send-email will be a good choice. CC list is
useless for patch it self, but here it will exist in commit log(although
this style can also be seen in linux kernel)

Thanks,
Michael
>>> --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
>>> +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
>>> @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
>>>  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
>>>  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
>>>  			if (cmdline_parse_etheraddr(NULL,
>>> -							pair[1],
>>> -							&dict->addr) < 0) {
>>> +						    pair[1],
>>> +						    &dict->addr,
>>> +						    sizeof(struct ether_addr)) < 0) {
>> Why not sizeof(dict->addr)?
>>
> Because addr is a struct ether_addr, and I always get confused when doing sizeof
> on pointer variables, so I find it more clear to specify the type exactly.  I'm
> not bound to it though so if you like I can change it, though given its release
> day, I figure you want to fix this build break asap.
> Neil
>
>> -- 
>> Thomas
>>

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Thu, Dec 18, 2014 at 01:51:13PM +0000, Qiu, Michael wrote:
> On 2014/12/18 19:26, Neil Horman wrote:
> > On Wed, Dec 17, 2014 at 11:20:26PM +0100, Thomas Monjalon wrote:
> >> Hi Neil,
> >>
> >> 2014-12-17 12:03, Neil Horman:
> >>> Back in:
> >>>
> >>> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> >>> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >>> Date:   Fri Dec 5 15:19:07 2014 +0100
> >>>
> >>>     cmdline: fix overflow on bsd
> >>>
> >>> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> >>> patch makes the needed correction to avoid a build break
> >>>
> >>> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> >>> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> >> What is the meaning of CC here?
> >>
> > CC is a tag that git send-email understands.  As it implies it cc's the post to
> > the indicated email, and records that fact in the body of the commit.
> 
> But if you use --cc in git send-email will be a good choice. CC list is
> useless for patch it self, but here it will exist in commit log(although
> this style can also be seen in linux kernel)
Yes, its good to have a record of who was CCed on a patch for archival purposes,
so you can get an idea of who participated (or was expected to participate in a
review)

Neil

> 
> Thanks,
> Michael
> >>> --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> >>> +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> >>> @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
> >>>  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
> >>>  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
> >>>  			if (cmdline_parse_etheraddr(NULL,
> >>> -							pair[1],
> >>> -							&dict->addr) < 0) {
> >>> +						    pair[1],
> >>> +						    &dict->addr,
> >>> +						    sizeof(struct ether_addr)) < 0) {
> >> Why not sizeof(dict->addr)?
> >>
> > Because addr is a struct ether_addr, and I always get confused when doing sizeof
> > on pointer variables, so I find it more clear to specify the type exactly.  I'm
> > not bound to it though so if you like I can change it, though given its release
> > day, I figure you want to fix this build break asap.
> > Neil
> >
> >> -- 
> >> Thomas
> >>
> 
> 

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Cao, Waterman]

Hi Neil,

	Can you let me know what configuration you used for XEN Domain U?
	Do you able to run some test cases on XEN Domain U?
	So far, we met some issues on xenvirt (Domain U). 

	Thanks

Waterman 

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Thu, Dec 18, 2014 at 03:40:54AM +0000, Cao, Waterman wrote:
> Hi Neil,
> 
> 	Can you let me know what configuration you used for XEN Domain U?
> 	Do you able to run some test cases on XEN Domain U?
> 	So far, we met some issues on xenvirt (Domain U). 
> 
> 	Thanks
> 
> Waterman 
> 
No configuration, I simply turned on the option in the build to ensure another
patch set of mine didn't break anything.
Neil

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Olivier MATZ]

Hi Neil,

On 12/17/2014 06:03 PM, Neil Horman wrote:
> Back in:
> 
> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date:   Fri Dec 5 15:19:07 2014 +0100
> 
>     cmdline: fix overflow on bsd
> 
> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> patch makes the needed correction to avoid a build break
> 
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

Sorry, I missed that one, thank you for fixing it.

Acked-by: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Olivier MATZ]

Hi,

On 12/18/2014 09:40 AM, Olivier MATZ wrote:
> Hi Neil,
> 
> On 12/17/2014 06:03 PM, Neil Horman wrote:
>> Back in:
>>
>> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
>> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Date:   Fri Dec 5 15:19:07 2014 +0100
>>
>>     cmdline: fix overflow on bsd
>>
>> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
>> patch makes the needed correction to avoid a build break
>>
>> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> 
> Sorry, I missed that one, thank you for fixing it.
> 
> Acked-by: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

Hmm, I agree with Thomas that sizeof(dict->addr) would be better
as it explicitly uses the length of the field we write into.

Regards,
Olivier

[PATCH v2] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

Back in:

commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date:   Fri Dec 5 15:19:07 2014 +0100

    cmdline: fix overflow on bsd

The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
patch makes the needed correction to avoid a build break

Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
CC: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

---
Change notes

v2: Changed sizeof(struct ether_addr) to sizeof(dict->addr)
---
 lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
index 6555ec5..04e30c9 100644
--- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
+++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
@@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
 		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
 				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
 			if (cmdline_parse_etheraddr(NULL,
-							pair[1],
-							&dict->addr) < 0) {
+						    pair[1],
+						    &dict->addr,
+						    sizeof(dict->addr)) < 0) {
 				RTE_LOG(ERR, PMD,
 					"Invalid %s device ether address\n",
 					name);
-- 
1.9.3

Re: [PATCH v2] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Qiu, Michael]

Hi Neil,

I think if you could add the commit author in the cc list will be better.

Because this could let him know about his code's issue and he is the
always the best person to review the patch.

Thanks,
Michael

On 2014/12/18 19:32, Neil Horman wrote:
> Back in:
>
> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date:   Fri Dec 5 15:19:07 2014 +0100
>
>     cmdline: fix overflow on bsd
>
> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> patch makes the needed correction to avoid a build break
>
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> CC: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
>
> ---
> Change notes
>
> v2: Changed sizeof(struct ether_addr) to sizeof(dict->addr)
> ---
>  lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> index 6555ec5..04e30c9 100644
> --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
>  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
>  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
>  			if (cmdline_parse_etheraddr(NULL,
> -							pair[1],
> -							&dict->addr) < 0) {
> +						    pair[1],
> +						    &dict->addr,
> +						    sizeof(dict->addr)) < 0) {
>  				RTE_LOG(ERR, PMD,
>  					"Invalid %s device ether address\n",
>  					name);

Re: [PATCH v2] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Thu, Dec 18, 2014 at 01:45:54PM +0000, Qiu, Michael wrote:
> Hi Neil,
> 
> I think if you could add the commit author in the cc list will be better.
> 
The commit author is me, and its recorded by the Signed-off line, as well as the
Authorship tag in git.  Not really sure what you're driving at here.

> Because this could let him know about his code's issue and he is the
> always the best person to review the patch.
> 
Yes, Git already does that, as noted above.

Neil


> Thanks,
> Michael
> 
> On 2014/12/18 19:32, Neil Horman wrote:
> > Back in:
> >
> > commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> > Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Date:   Fri Dec 5 15:19:07 2014 +0100
> >
> >     cmdline: fix overflow on bsd
> >
> > The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> > patch makes the needed correction to avoid a build break
> >
> > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> > CC: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> >
> > ---
> > Change notes
> >
> > v2: Changed sizeof(struct ether_addr) to sizeof(dict->addr)
> > ---
> >  lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> > index 6555ec5..04e30c9 100644
> > --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> > +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
> > @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
> >  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
> >  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
> >  			if (cmdline_parse_etheraddr(NULL,
> > -							pair[1],
> > -							&dict->addr) < 0) {
> > +						    pair[1],
> > +						    &dict->addr,
> > +						    sizeof(dict->addr)) < 0) {
> >  				RTE_LOG(ERR, PMD,
> >  					"Invalid %s device ether address\n",
> >  					name);
> 
> 

Re: [PATCH v2] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Qiu, Michael]

On 12/19/2014 4:54 AM, Neil Horman wrote:
> On Thu, Dec 18, 2014 at 01:45:54PM +0000, Qiu, Michael wrote:
>> Hi Neil,
>>
>> I think if you could add the commit author in the cc list will be better.
>>
> The commit author is me, and its recorded by the Signed-off line, as well as the
> Authorship tag in git.  Not really sure what you're driving at here.

Sorry, What I mean "commit author" is who mentioned in you commit log,
for this case should be this guy: alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org I think.
Sorry for I did not say it clearly.

Yourself, of course, should be in the list.

Thanks,
Michael
>
>> Because this could let him know about his code's issue and he is the
>> always the best person to review the patch.
>>
> Yes, Git already does that, as noted above.
>
> Neil
>
>
>> Thanks,
>> Michael
>>
>> On 2014/12/18 19:32, Neil Horman wrote:
>>> Back in:
>>>
>>> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
>>> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>> Date:   Fri Dec 5 15:19:07 2014 +0100
>>>
>>>     cmdline: fix overflow on bsd
>>>
>>> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
>>> patch makes the needed correction to avoid a build break
>>>
>>> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
>>> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
>>> CC: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
>>>
>>> ---
>>> Change notes
>>>
>>> v2: Changed sizeof(struct ether_addr) to sizeof(dict->addr)
>>> ---
>>>  lib/librte_pmd_xenvirt/rte_eth_xenvirt.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
>>> index 6555ec5..04e30c9 100644
>>> --- a/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
>>> +++ b/lib/librte_pmd_xenvirt/rte_eth_xenvirt.c
>>> @@ -586,8 +586,9 @@ rte_eth_xenvirt_parse_args(struct xenvirt_dict *dict,
>>>  		if (!strncmp(pair[0], RTE_ETH_XENVIRT_MAC_PARAM,
>>>  				sizeof(RTE_ETH_XENVIRT_MAC_PARAM))) {
>>>  			if (cmdline_parse_etheraddr(NULL,
>>> -							pair[1],
>>> -							&dict->addr) < 0) {
>>> +						    pair[1],
>>> +						    &dict->addr,
>>> +						    sizeof(dict->addr)) < 0) {
>>>  				RTE_LOG(ERR, PMD,
>>>  					"Invalid %s device ether address\n",
>>>  					name);
>>

Re: [PATCH v2] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Thomas Monjalon]

2014-12-18 06:31, Neil Horman:
> Back in:
> 
> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date:   Fri Dec 5 15:19:07 2014 +0100
> 
>     cmdline: fix overflow on bsd
> 
> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> patch makes the needed correction to avoid a build break
> 
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> CC: Olivier Matz <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> ---
> Change notes
> 
> v2: Changed sizeof(struct ether_addr) to sizeof(dict->addr)

Applied

Thanks
-- 
Thomas

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Stephen Hemminger]

On Wed, 17 Dec 2014 12:03:28 -0500
Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:

> Back in:
> 
> commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date:   Fri Dec 5 15:19:07 2014 +0100
> 
>     cmdline: fix overflow on bsd
> 
> The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> patch makes the needed correction to avoid a build break
> 
> Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>

If we could fix the header incompatiablities then the driver could use
a standard library like ether_aton() instead of dragging in the unnecessary
cmdline library.

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Thomas Monjalon]

2014-12-18 08:17, Stephen Hemminger:
> On Wed, 17 Dec 2014 12:03:28 -0500
> Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
[...]
> > The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> > patch makes the needed correction to avoid a build break
[...]
> 
> If we could fix the header incompatiablities then the driver could use
> a standard library like ether_aton() instead of dragging in the unnecessary
> cmdline library.

Right.
A first fix was done to remove conflict with netinet/in.h:
	http://dpdk.org/browse/dpdk/commit/?id=d07180f211
But there still are conflicts with netinet/ether.h and maybe more.
I suggest to fix it in the release 2.0.

-- 
Thomas

Re: [PATCH] xenvirt: Fix build break on cmdline_parse_etheraddr call

[Neil Horman]

On Thu, Dec 18, 2014 at 08:17:12AM -0800, Stephen Hemminger wrote:
> On Wed, 17 Dec 2014 12:03:28 -0500
> Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org> wrote:
> 
> > Back in:
> > 
> > commit aaa662e75c23c61a1d79bd4d1f9f35b4967c39db
> > Author: Alan Carew <alan.carew-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Date:   Fri Dec 5 15:19:07 2014 +0100
> > 
> >     cmdline: fix overflow on bsd
> > 
> > The author failed to fixup a call to cmdline_parse_etheraddr in xenvirt.  This
> > patch makes the needed correction to avoid a build break
> > 
> > Signed-off-by: Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> > CC: Thomas Monjalon <thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
> 
> If we could fix the header incompatiablities then the driver could use
> a standard library like ether_aton() instead of dragging in the unnecessary
> cmdline library.
> 
I agree, that would be great, but I think that would be better done after the
release, since this is in compliance with how the rest of the DPDK handles this
situation currently.  Using ether_aton is definately a superior solution
however.

Neil