From: Stephen Hemminger <stephen@networkplumber.org>
To: Aaron Conole <aconole@redhat.com>
Cc: dev@dpdk.org, Olivier Matz <olivier.matz@6wind.com>,
Andrew Rybchenko <arybchenko@solarflare.com>,
Ferruh Yigit <ferruh.yigit@intel.com>,
Michael Santana <msantana@redhat.com>
Subject: Re: [dpdk-dev] [PATCH] rte_ether: force format string for unformat_addr
Date: Wed, 10 Jul 2019 12:27:56 -0700 [thread overview]
Message-ID: <20190710122756.62ec19a9@hermes.lan> (raw)
In-Reply-To: <f7ta7dlog8x.fsf@dhcp-25.97.bos.redhat.com>
On Wed, 10 Jul 2019 15:13:02 -0400
Aaron Conole <aconole@redhat.com> wrote:
> Stephen Hemminger <stephen@networkplumber.org> writes:
>
> > On Wed, 10 Jul 2019 14:33:42 -0400
> > Aaron Conole <aconole@redhat.com> wrote:
> >
> >> rte_ether_unformation_addr is very lax in what it accepts now, including
> >> ethernet addresses formatted ambiguously as "x:xx:x:xx:x:xx". However,
> >> previously this behavior was enforced via the my_ether_aton which would
> >> fail ambiguously formatted values.
> >>
> >> Reported-by: Michael Santana <msantana@redhat.com>
> >> Fixes: 596d31092d32 ("net: add function to convert string to ethernet address")
> >> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> ---
> >> lib/librte_net/rte_ether.c | 6 ++++--
> >> 1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_net/rte_ether.c b/lib/librte_net/rte_ether.c
> >> index 8d040173c..4f252b813 100644
> >> --- a/lib/librte_net/rte_ether.c
> >> +++ b/lib/librte_net/rte_ether.c
> >> @@ -45,7 +45,8 @@ rte_ether_unformat_addr(const char *s, struct rte_ether_addr *ea)
> >> if (n == 6) {
> >> /* Standard format XX:XX:XX:XX:XX:XX */
> >> if (o0 > UINT8_MAX || o1 > UINT8_MAX || o2 > UINT8_MAX ||
> >> - o3 > UINT8_MAX || o4 > UINT8_MAX || o5 > UINT8_MAX) {
> >> + o3 > UINT8_MAX || o4 > UINT8_MAX || o5 > UINT8_MAX ||
> >> + strlen(s) != RTE_ETHER_ADDR_FMT_SIZE - 1) {
> >> rte_errno = ERANGE;
> >> return -1;
> >> }
> >> @@ -58,7 +59,8 @@ rte_ether_unformat_addr(const char *s, struct rte_ether_addr *ea)
> >> ea->addr_bytes[5] = o5;
> >> } else if (n == 3) {
> >> /* Support the format XXXX:XXXX:XXXX */
> >> - if (o0 > UINT16_MAX || o1 > UINT16_MAX || o2 > UINT16_MAX) {
> >> + if (o0 > UINT16_MAX || o1 > UINT16_MAX || o2 > UINT16_MAX ||
> >> + strlen(s) != RTE_ETHER_ADDR_FMT_SIZE - 4) {
> >> rte_errno = ERANGE;
> >> return -1;
> >> }
> >
> > NAK
> > Skipping leading zero should be ok. There is no need for this patch.
>
> Is it intended to skip the leading 0? Why not the trailing 0? I'm not
> familiar with the format that is used here (example - X:XX:X:XX:X)
>
> It isn't described in any RFC I could find (but I only did a small
> search). Even in IEEE, the format is always a full octet.
>
> > The current behavior is superset of what standard ether_aton accepts.
>
> Okay, but it introduces a test failure for the cmdline tests and then
> that test will need a few lines removed for 'unsuccessful' formats.
>
> ether_aton is much more rigid in the formats it accepts, so the test
> case is enforcing that. I guess either the current behavior of this
> function changes (and since it is a new behavior of the cmdline parser,
> I would think it should be changed) or the test case should be changed
> to adopt it.
BSD ether_aton is:
/*
* Convert an ASCII representation of an ethernet address to binary form.
*/
struct ether_addr *
ether_aton_r(const char *a, struct ether_addr *e)
{
int i;
unsigned int o0, o1, o2, o3, o4, o5;
i = sscanf(a, "%x:%x:%x:%x:%x:%x", &o0, &o1, &o2, &o3, &o4, &o5);
if (i != 6)
return (NULL);
e->octet[0]=o0;
e->octet[1]=o1;
e->octet[2]=o2;
e->octet[3]=o3;
e->octet[4]=o4;
e->octet[5]=o5;
return (e);
}
next prev parent reply other threads:[~2019-07-10 19:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-10 18:33 Aaron Conole
2019-07-10 18:42 ` Stephen Hemminger
2019-07-10 19:13 ` Aaron Conole
2019-07-10 19:27 ` Stephen Hemminger [this message]
2019-07-10 20:31 ` Aaron Conole
2019-07-10 23:13 ` Stephen Hemminger
2019-07-17 18:42 ` Stephen Hemminger
2019-07-19 17:59 ` Ferruh Yigit
2019-07-21 17:32 ` Aaron Conole
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190710122756.62ec19a9@hermes.lan \
--to=stephen@networkplumber.org \
--cc=aconole@redhat.com \
--cc=arybchenko@solarflare.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=msantana@redhat.com \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).