rte_ether_unformat: accept more inputs
Subsystems (2)
RFC 1/1 rte_ether_unformat: accept more inputs
This updates rte_ether_addr_unformat() to accept more types of input. There have been requests to handle Windows and other formats.
---
Marking this as RFC until unit tests are added.
lib/net/rte_ether.c | 78 +++++++++++++++++++++++++++++++++++---------- lib/net/rte_ether.h | 6 ++-- 2 files changed, 66 insertions(+), 18 deletions(-)
lib/net/rte_ether.c +62 -16
@@ -38,7 +38,8 @@ static int8_t get_xdigit(char ch) }/* Convert 00:11:22:33:44:55 to ethernet address */ -static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea) +static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea, +const char sep) {const char *s = s0;int i;
@@ -50,14 +51,17 @@ static bool get_ether_addr6(const char *s0, struct rte_ether_addr *ea)if (x <0 )return false; - ea->addr_bytes[i] = x <<4 ; - x =get_xdigit (*s++); -if (x <0 ) -return false; - ea->addr_bytes[i] |= x; + ea->addr_bytes[i] = x; +if (*s != sep) { + x =get_xdigit (*s++); +if (x <0 ) +return false; + ea->addr_bytes[i] <<=4 ; + ea->addr_bytes[i] |= x; + }if (i <RTE_ETHER_ADDR_LEN -1 && - *s++ !=':' ) + *s++ != sep)return false; }
@@ -66,7 +70,8 @@ static bool get_ether_addr6(const char *s0, struct rte_ether_addr *ea) }/* Convert 0011:2233:4455 to ethernet address */ -static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea) +static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea, +const char sep) {int i, j;
@@ -80,12 +85,14 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea)if (x <0 )return false; w = (w <<4 ) | x; +if (*s == sep) +break ; } ea->addr_bytes[i] = w >>8 ; ea->addr_bytes[i +1 ] = w &0xff ;if (i <RTE_ETHER_ADDR_LEN -2 && - *s++ !=':' ) + *s++ != sep)return false; }
@@ -93,17 +100,56 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea) }/* -* Like ether_aton_r but can handle either -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX -* and is more restrictive. +* Scan input to see if seperated by dash, colon or period +* Returns seperator and number of matches +* If seperators are mixed will return +*/ +static unsigned int get_ether_sep (const char *s,char *sep) +{ +const char seperators[] ="-:." ; +unsigned int count =0 ; +const char *cp; + + cp =strpbrk (s, seperators); +if (cp ==NULL ) +return 0 ; + + *sep = *cp; +do { + ++count; +/* find next instance of seperator */ + cp =strchr (cp +1 , *sep); + }while (cp !=NULL ); + +return count; +} + +/* +* Be libreal in accepting a wide variety of notational formats +* for MAC address including: +* - Linux format six groups of hexadecimal digits seperated by colon +* - Windows format six groups seperated by hyphen +* - two groups hexadecimal digits */ int rte_ether_unformat_addr (const char *s,struct rte_ether_addr *ea) { -if (get_ether_addr6 (s, ea)) -return 0 ; -if (get_ether_addr3 (s, ea)) -return 0 ; +unsigned int count; +char sep ='\0' ; + + count =get_ether_sep (s, &sep); +switch (count) { +case 5 :/* i.e 01:23:45:67:89:AB */ +if (get_ether_addr6 (s, ea, sep)) +return 0 ; +break ; +case 2 :/* i.e 0123.4567.89AB */ +if (get_ether_addr3 (s, ea, sep)) +return 0 ; +break ; +default : +break ; + } rte_errno =EINVAL ;return -1 ;
lib/net/rte_ether.h +4 -2
@@ -254,8 +254,10 @@ rte_ether_format_addr(char *buf, uint16_t size,* * @param str * A pointer to buffer contains the formatted MAC address. -* The supported formats are: -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX +* The example formats are: +* XX:XX:XX:XX:XX:XX - Canonical form +* XX-XX-XX-XX-XX-XX - Windows and IEEE 802 +* XXXX:XXXX:XXXX - original DPDK * where XX is a hex digit: 0-9, a-f, or A-F. * @param eth_addr * A pointer to a ether_addr structure.
Signature · 1 line
2.39.2
Quoted 5 lines
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, 29 September 2023 18.36This updates
rte_ether_addr_unformat()to accept more types of input. There have been requests to handle Windows and other formats.
Seems very useful.
[...]
Quoted 2 lines
+/*
+ * Be libreal in accepting a wide variety of notational formats
Typo: libreal -> liberal.
Quoted 3 lines
+ * for MAC address including:
+ * - Linux format six groups of hexadecimal digits seperated by colon
+ * - Windows format six groups seperated by hyphen
Typo (twice): seperated -> separated
[...]
lib/net/rte_ether.h rte_ether_format_addr(char *buf, uint16_t size, · line 261
Quoted 13 lines
lib/net/rte_ether.h +4
-2@@ -254,8 +254,10 @@ rte_ether_format_addr(char *buf, uint16_t size,* * @param str * A pointer to buffer contains the formatted MAC address. -* The supported formats are: -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX +* The example formats are: +* XX:XX:XX:XX:XX:XX - Canonical form +* XX-XX-XX-XX-XX-XX - Windows and IEEE 802 +* XXXX:XXXX:XXXX - original DPDK * where XX is a hex digit: 0-9, a-f, or A-F.
Actually, XX is a hex number, not a hex digit.
But I think the intention of this patch is also to allow a hex number of 1 or 2 (or up to 4) hex digits for XX and XXXX?
Quoted 13 lines · On Fri, 29 Sep 2023 21:35:45 +0200 Morten Brørup <[email protected]>…
Quoted 5 lines
From: Stephen Hemminger [mailto:[email protected]]
Sent: Friday, 29 September 2023 18.36This updates
rte_ether_addr_unformat()to accept more types of input. There have been requests to handle Windows and other formats.Seems very useful.
[...]
Quoted 2 lines
+/*
+ * Be libreal in accepting a wide variety of notational formats
Quoted 7 lines
Typo: libreal -> liberal.
Quoted 3 lines
+ * for MAC address including:
+ * - Linux format six groups of hexadecimal digits seperated by colon
+ * - Windows format six groups seperated by hyphenTypo (twice): seperated -> separated
Typos will be fixed in next version in addition to adding tests.
Quoted 17 lines
Quoted 4 lines
lib/net/rte_ether.h rte_ether_format_addr(char *buf, uint16_t size, · line 261
Quoted 10 lines
lib/net/rte_ether.h +4
-2@@ -254,8 +254,10 @@ rte_ether_format_addr(char *buf, uint16_t size,* * @param str * A pointer to buffer contains the formatted MAC address. -* The supported formats are: -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX +* The example formats are: +* XX:XX:XX:XX:XX:XX - Canonical form +* XX-XX-XX-XX-XX-XX - Windows and IEEE 802 +* XXXX:XXXX:XXXX - original DPDK * where XX is a hex digit: 0-9, a-f, or A-F. Actually, XX is a hex number, not a hex digit.
Quoted 2 lines
But I think the intention of this patch is also to allow a hex number of 1 or 2 (or up to 4) hex digits for XX and XXXX?
Good point, the single hex digit format came as a later request.
v2 cover rte_ether_unformat_addr changes
In reply to Stephen HemmingerThis patchset makes rte_ether_unformat_addr allow other formats for MAC address. Need to remove some inputs from existing cmdline_etheraddr test, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.
Stephen Hemminger (3):
test: remove some strings from cmdline_etheraddr tests
rte_ether_unformat: accept more inputs
test: add tests for rte_ether routines
app/test/meson.build | 1 + app/test/test_cmdline_etheraddr.c | 11 +- app/test/test_net_ether.c | 169 ++++++++++++++++++++++++++++++ lib/net/rte_ether.c | 87 +++++++++++---- lib/net/rte_ether.h | 9 +- 5 files changed, 248 insertions(+), 29 deletions(-) create mode 100644 app/test/test_net_ether.c
Signature · 1 line
2.39.2
PATCH v2 1/3 test: remove some strings from cmdline_etheraddr tests
Some of the ethernet address formats which were invalid will now become valid inputs when rte_ether_unformat_addr is modified to allow leading zeros.
Also, make local variables static.
--- app/test/test_cmdline_etheraddr.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
app/test/test_cmdline_etheraddr.c +4 -7
@@ -20,7 +20,7 @@ struct ether_addr_str { };/* valid strings */ -const struct ether_addr_str ether_addr_valid_strs[] = { +static const struct ether_addr_str ether_addr_valid_strs[] = { {"01:23:45:67:89:AB" ,0xAB8967452301ULL }, {"4567:89AB:CDEF" ,0xEFCDAB896745ULL }, };
@@ -30,7 +30,7 @@ const struct ether_addr_str ether_addr_valid_strs[] = {* end of token, which is either space chars, null char or * a hash sign. */ -const char * ether_addr_garbage_strs[] = { +static const char *const ether_addr_garbage_strs[] = {"00:11:22:33:44:55\0garbage" ,"00:11:22:33:44:55#garbage" ,"00:11:22:33:44:55 garbage" ,
@@ -46,14 +46,13 @@ const char * ether_addr_garbage_strs[] = {#define GARBAGE_ETHERADDR 0x554433221100ULL/* corresponding address */ -const char * ether_addr_invalid_strs[] = { +static const char *const ether_addr_invalid_strs[] = {/* valid chars, invalid syntax */ "0123:45:67:89:AB" ,"01:23:4567:89:AB" ,"01:23:45:67:89AB" ,"012:345:678:9AB" ,"01:23:45:67:89:ABC" , -"01:23:45:67:89:A" ,"01:23:45:67:89" ,"01:23:45:67:89:AB:CD" ,/* invalid chars, valid syntax */
@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" ,"01,23,45,67,89,AB" , -"01:23:45\0:67:89:AB" , -"01:23:45#:67:89:AB" , +"01:23:45:67#:89:AB" ,"random invalid text" ,"random text" , "",
Signature · 1 line
2.39.2
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
Quoted 8 lines · Stephen Hemminger
Some of the ethernet address formats which were invalid will now become valid inputs when
rte_ether_unformat_addris modified to allow leading zeros.Also, make local variables static.
Signed-off-by Stephen Hemminger <[email protected]>
<...>
app/test/test_cmdline_etheraddr.c const char * ether_addr_invalid_strs[] = { · line 64
Quoted 9 lines
app/test/test_cmdline_etheraddr.c +0
-3@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" , "01,23,45,67,89,AB", -"01:23:45\0:67:89:AB" , - "01:23:45#:67:89:AB",
Why these two are valid now?
And I guess second one is still not valid, but first one is parsed as [1], is this expected?
[1] 00:01:00:23:00:45
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
Quoted 24 lines · Ferruh Yigit
Quoted 8 lines · Stephen Hemminger
Some of the ethernet address formats which were invalid will now become valid inputs when
rte_ether_unformat_addris modified to allow leading zeros.Also, make local variables static.
Signed-off-by Stephen Hemminger <[email protected]><...>
app/test/test_cmdline_etheraddr.c const char * ether_addr_invalid_strs[] = { · line 64
Quoted 9 lines
app/test/test_cmdline_etheraddr.c +0
-3@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" , "01,23,45,67,89,AB", -"01:23:45\0:67:89:AB" , - "01:23:45#:67:89:AB",Why these two are valid now?
Quoted 6 lines
And I guess second one is still not valid, but first one is parsed as [1], is this expected?
[1] 00:01:00:23:00:45
Ah, I guess it is taken as "XXXX:XXXX:XXXX" format, but number of digit is not enforced, so "1:2:3" is a valid format, should we add this to API documentation as example format? Or is this unintended side effect?
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
Quoted 3 lines · On Tue, 3 Oct 2023 11:59:04 +0100 Ferruh Yigit <[email protected]> wrote:
Ah, I guess it is taken as "XXXX:XXXX:XXXX" format, but number of digit is not enforced, so "1:2:3" is a valid format, should we add this to API documentation as example format? Or is this unintended side effect?
By allowing leading zeros, it becomes allowed.
DPDK always allowed the non-standard 3 part format. Looking around only old Cisco ACS used 3 part MAC format and it used periods.
The API documentation should mention leading zeros are optional.
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
In reply to Ferruh YigitQuoted 24 lines · On Tue, 3 Oct 2023 11:47:51 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 8 lines · Stephen Hemminger
Some of the ethernet address formats which were invalid will now become valid inputs when
rte_ether_unformat_addris modified to allow leading zeros.Also, make local variables static.
Signed-off-by Stephen Hemminger <[email protected]><...>
app/test/test_cmdline_etheraddr.c const char * ether_addr_invalid_strs[] = { · line 64
Quoted 9 lines
app/test/test_cmdline_etheraddr.c +0
-3@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" , "01,23,45,67,89,AB", -"01:23:45\0:67:89:AB" , - "01:23:45#:67:89:AB",Why these two are valid now?
Quoted 4 lines
And I guess second one is still not valid, but first one is parsed as [1], is this expected?
[1] 00:01:00:23:00:45
The code in cmdline converts the comment character # to a null byte. So both test are the same.
With new unformat, it allows a 3 part mac address with leading zeros.
01:23:45 is equivalent to 0001:0023:0045
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
Quoted 30 lines · Stephen Hemminger
Quoted 27 lines · On Tue, 3 Oct 2023 11:47:51 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 8 lines · Stephen Hemminger
Some of the ethernet address formats which were invalid will now become valid inputs when
rte_ether_unformat_addris modified to allow leading zeros.Also, make local variables static.
Signed-off-by Stephen Hemminger <[email protected]><...>
app/test/test_cmdline_etheraddr.c const char * ether_addr_invalid_strs[] = { · line 64
Quoted 9 lines
app/test/test_cmdline_etheraddr.c +0
-3@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" , "01,23,45,67,89,AB", -"01:23:45\0:67:89:AB" , - "01:23:45#:67:89:AB",Why these two are valid now?
And I guess second one is still not valid, but first one is parsed as [1], is this expected?
Quoted 9 lines
Quoted 1 line
[1] 00:01:00:23:00:45
The code in cmdline converts the comment character # to a null byte. So both test are the same.
With new unformat, it allows a 3 part mac address with leading zeros.
01:23:45 is equivalent to 0001:0023:0045
With 3 part MAC, omitting leading zeros looks confusing to me, because that omitted part becomes an octet in MAC. Like:
01:23:45 being equivalent to 00:01:00:23:00:45
As 3 part MAC, and 6 part MAC has separate functions, does it makes sense to require leading zeros in the 3 part MAC, what do you think?
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
Quoted 41 lines · On Tue, 3 Oct 2023 17:50:13 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 30 lines · Stephen Hemminger
Quoted 27 lines · On Tue, 3 Oct 2023 11:47:51 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 8 lines · Stephen Hemminger
Some of the ethernet address formats which were invalid will now become valid inputs when
rte_ether_unformat_addris modified to allow leading zeros.Also, make local variables static.
Signed-off-by Stephen Hemminger <[email protected]><...>
app/test/test_cmdline_etheraddr.c const char * ether_addr_invalid_strs[] = { · line 64
Quoted 9 lines
app/test/test_cmdline_etheraddr.c +0
-3@@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" , "01,23,45,67,89,AB", -"01:23:45\0:67:89:AB" , - "01:23:45#:67:89:AB",Why these two are valid now?
And I guess second one is still not valid, but first one is parsed as [1], is this expected?
Quoted 9 lines
Quoted 1 line
[1] 00:01:00:23:00:45
The code in cmdline converts the comment character # to a null byte. So both test are the same.
With new unformat, it allows a 3 part mac address with leading zeros.
01:23:45 is equivalent to 0001:0023:0045
Quoted 7 lines
With 3 part MAC, omitting leading zeros looks confusing to me, because that omitted part becomes an octet in MAC. Like:
01:23:45 being equivalent to 00:01:00:23:00:45As 3 part MAC, and 6 part MAC has separate functions, does it makes sense to require leading zeros in the 3 part MAC, what do you think?
Right, 3 part MAC is only a legacy Cisco format, not used anywhere else.
PATCH v2 2/3 rte_ether_unformat: accept more inputs
In reply to Stephen HemmingerThis updates rte_ether_addr_unformat() to accept more types formats for MAC address. It allows IEEE, IETF and Cisco formats with leading zeros as well.
--- lib/net/rte_ether.c | 87 ++++++++++++++++++++++++++++++++++----------- lib/net/rte_ether.h | 9 +++-- 2 files changed, 74 insertions(+), 22 deletions(-)
lib/net/rte_ether.c +67 -20
@@ -38,7 +38,8 @@ static int8_t get_xdigit(char ch) }/* Convert 00:11:22:33:44:55 to ethernet address */ -static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea) +static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea, +const char sep) {const char *s = s0;int i;
@@ -48,25 +49,29 @@ static bool get_ether_addr6(const char *s0, struct rte_ether_addr *ea) x =get_xdigit (*s++);if (x <0 ) -return false; +return false;/* not a hex digit */ - ea->addr_bytes[i] = x <<4 ; - x =get_xdigit (*s++); -if (x <0 ) -return false; - ea->addr_bytes[i] |= x; + ea->addr_bytes[i] = x; +if (*s != sep && *s !='\0' ) { + x =get_xdigit (*s++); +if (x <0 ) +return false;/* not a hex digit */ + ea->addr_bytes[i] <<=4 ; + ea->addr_bytes[i] |= x; + }if (i <RTE_ETHER_ADDR_LEN -1 && - *s++ !=':' ) -return false; + *s++ != sep) +return false;/* premature end of string */ } -/* return true if at end of string */ +/* return true if no trailing characters */ return *s =='\0' ; }/* Convert 0011:2233:4455 to ethernet address */ -static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea) +static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea, +const char sep) {int i, j;
@@ -78,14 +83,17 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea) x =get_xdigit (*s++);if (x <0 ) -return false; +return false;/* not a hex digit */ w = (w <<4 ) | x; +if (*s == sep || *s =='\0' ) +break ; } + ea->addr_bytes[i] = w >>8 ; ea->addr_bytes[i +1 ] = w &0xff ;if (i <RTE_ETHER_ADDR_LEN -2 && - *s++ !=':' ) + *s++ != sep)return false; }
@@ -93,17 +101,56 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea) }/* -* Like ether_aton_r but can handle either -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX -* and is more restrictive. +* Scan input to see if separated by dash, colon or period +* Returns separator and number of matches +* If separators are mixed will return +*/ +static unsigned int get_ether_sep (const char *s,char *sep) +{ +static const char separators[] ="-:." ; +unsigned int count =0 ; +const char *cp; + + cp =strpbrk (s, separators); +if (cp ==NULL ) +return 0 ;/* no separator found */ + + *sep = *cp;/* return the separator */ +do { + ++count; +/* find next instance of separator */ + cp =strchr (cp +1 , *sep); + }while (cp !=NULL ); + +return count; +} + +/* +* Be liberal in accepting a wide variety of notational formats +* for MAC address including: +* - Linux format six groups of hexadecimal digits separated by colon +* - Windows format six groups separated by hyphen +* - two groups hexadecimal digits */ int rte_ether_unformat_addr (const char *s,struct rte_ether_addr *ea) { -if (get_ether_addr6 (s, ea)) -return 0 ; -if (get_ether_addr3 (s, ea)) -return 0 ; +unsigned int count; +char sep ='\0' ; + + count =get_ether_sep (s, &sep); +switch (count) { +case 5 :/* i.e 01:23:45:67:89:AB */ +if (get_ether_addr6 (s, ea, sep)) +return 0 ; +break ; +case 2 :/* i.e 0123.4567.89AB */ +if (get_ether_addr3 (s, ea, sep)) +return 0 ; +break ; +default : +break ; + } rte_errno =EINVAL ;return -1 ;
lib/net/rte_ether.h +7 -2
@@ -254,8 +254,13 @@ rte_ether_format_addr(char *buf, uint16_t size,* * @param str * A pointer to buffer contains the formatted MAC address. -* The supported formats are: -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX +* Accepts either byte or word format separated by colon, +* hyphen or period. +* +* The example formats are: +* XX:XX:XX:XX:XX:XX - Canonical form +* XX-XX-XX-XX-XX-XX - Windows and IEEE 802 +* XXXX.XXXX.XXXX - Cisco * where XX is a hex digit: 0-9, a-f, or A-F. * @param eth_addr * A pointer to a ether_addr structure.
Signature · 1 line
2.39.2
PATCH v2 3/3 test: add tests for rte_ether routines
In reply to Stephen HemmingerThis add some basic tests for rte_unformat_ether_addr and other functions in rte_ether.
--- app/test/meson.build | 1 + app/test/test_net_ether.c | 169 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+) create mode 100644 app/test/test_net_ether.c
app/test/meson.build +1 -0
@@ -124,6 +124,7 @@ source_file_deps = { 'test_meter.c': ['meter'], 'test_metrics.c': ['metrics'], 'test_mp_secondary.c': ['hash', 'lpm'], + 'test_net_ether.c': ['net'], 'test_pcapng.c': ['ethdev', 'net', 'pcapng'], 'test_pdcp.c': ['eventdev', 'pdcp', 'net', 'timer', 'security'], 'test_pdump.c': ['pdump'] + sample_packet_forward_deps,
app/test/test_net_ether.c new file +169 -0
169 lines not on this page · raw message
Signature · 1 line
2.39.2
RE: [PATCH v2 0/3] rte_ether_unformat_addr changes
In reply to Stephen HemmingerQuoted 2 lines
From: Stephen Hemminger [mailto:[email protected]]
Sent: Monday, 2 October 2023 20.37
] rte_ether_unformat_addr changes
Quoted 20 lines
This patchset makes
rte_ether_unformat_addrallow other formats for MAC address. Need to remove some inputs from existingcmdline_etheraddrtest, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.Stephen Hemminger (3):
test: remove some strings fromcmdline_etheraddrtests
rte_ether_unformat: accept more inputs
test: add tests forrte_etherroutinesapp/test/meson.build | 1 + app/test/test_cmdline_etheraddr.c | 11 +- app/test/test_net_ether.c | 169 ++++++++++++++++++++++++++++++ lib/net/rte_ether.c | 87 +++++++++++---- lib/net/rte_ether.h | 9 +- 5 files changed, 248 insertions(+), 29 deletions(-) create mode 100644 app/test/test_net_ether.c
Quoted 2 lines
--
2.39.2
Re: [PATCH v2 0/3] rte_ether_unformat_addr changes
In reply to Stephen HemmingerQuoted 11 lines · Stephen Hemminger
This patchset makes
rte_ether_unformat_addrallow other formats for MAC address. Need to remove some inputs from existingcmdline_etheraddrtest, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.Stephen Hemminger (3):
test: remove some strings fromcmdline_etheraddrtests
rte_ether_unformat: accept more inputs
test: add tests forrte_etherroutines
Thanks Stephen,
This enables using the API as replacement to the tap PMD's local parse implementation:
https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
Re: [PATCH v2 0/3] rte_ether_unformat_addr changes
Quoted 13 lines · On Tue, 3 Oct 2023 11:44:16 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 11 lines · Stephen Hemminger
This patchset makes
rte_ether_unformat_addrallow other formats for MAC address. Need to remove some inputs from existingcmdline_etheraddrtest, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.Stephen Hemminger (3):
test: remove some strings fromcmdline_etheraddrtests
rte_ether_unformat: accept more inputs
test: add tests forrte_etherroutines
Quoted 5 lines
Thanks Stephen,
This enables using the API as replacement to the tap PMD's local parse implementation:
https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
That looks good, but indentation is wrong and it duplicates a check for value == NULL which will cause a new Coverity warning.
Re: [PATCH v2 0/3] rte_ether_unformat_addr changes
In reply to Ferruh YigitQuoted 13 lines · On Tue, 3 Oct 2023 11:44:16 +0100 Ferruh Yigit <[email protected]> wrote:
Quoted 11 lines · Stephen Hemminger
This patchset makes
rte_ether_unformat_addrallow other formats for MAC address. Need to remove some inputs from existingcmdline_etheraddrtest, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.Stephen Hemminger (3):
test: remove some strings fromcmdline_etheraddrtests
rte_ether_unformat: accept more inputs
test: add tests forrte_etherroutines
Quoted 5 lines
Thanks Stephen,
This enables using the API as replacement to the tap PMD's local parse implementation:
https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
It can be simplified to just this.
No need to check value == NULL, already checkd.
No need to check for user_mac == NULL, since only called one place and that place passes pointer to stack variable.
From b478a17f13a1bedadca3b60608c585f31c9ad8f2 Mon Sep 17 00:00:00 2001
From: David Christensen <[email protected]>
Date: Thu, 23 Mar 2023 13:01:45 -0400
Subject: [PATCH] net/tap: resolve stringop-overflow with gcc 12 on ppc64le
Building DPDK with gcc 12 on a ppc64le system generates a stringop-overflow warning. Replace the local MAC address validation function parse_user_mac() with a call to rte_ether_unformat_addr() instead.
Bugzilla ID: 1197
Cc: [email protected]
--- drivers/net/tap/rte_eth_tap.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-)
drivers/net/tap/rte_eth_tap.c +1 -24
@@ -2267,29 +2267,6 @@ set_remote_iface(const char *key __rte_unused,return 0 ; } -static int parse_user_mac (struct rte_ether_addr *user_mac, -const char *value) -{ -unsigned int index =0 ; -char mac_temp[strlen (ETH_TAP_USR_MAC_FMT ) +1 ], *mac_byte =NULL ; - -if (user_mac ==NULL || value ==NULL ) -return 0 ; - -strlcpy (mac_temp, value,sizeof (mac_temp)); - mac_byte =strtok (mac_temp,":" ); - -while ((mac_byte !=NULL ) && - (strlen (mac_byte) <=2 ) && - (strlen (mac_byte) ==strspn (mac_byte, -ETH_TAP_CMP_MAC_FMT ))) { - user_mac->addr_bytes[index++] =strtoul (mac_byte,NULL ,16 ); - mac_byte =strtok (NULL ,":" ); - } - -return index; -} - static int set_mac_type(const char *key __rte_unused, const char *value,
@@ -2311,7 +2288,7 @@ set_mac_type(const char *key __rte_unused, goto success; } -if (parse_user_mac (user_mac, value) !=6 ) +if (rte_ether_unformat_addr (value, user_mac) <0 ) goto error; success:TAP_LOG (DEBUG ,"TAP user MAC param (%s)" , value);
Signature · 1 line
2.39.2
v3 cover rte_ether_unformat_addr related changes
In reply to Stephen HemmingerThis patchset makes rte_ether_unformat_addr allow other formats for MAC address. Need to remove some inputs from existing cmdline_etheraddr test, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.
v4 - fix spelling errors
don't allow leading zeros in Cisco 3 part format incorporate tap patch to use unformat addr
David Christensen (1):
net/tap: use rte_ether_unformat_address
Stephen Hemminger (3):
test: remove some strings from cmdline_etheraddr tests
rte_ether_unformat: accept more inputs
test: add tests for rte_ether routines
app/test/meson.build | 1 + app/test/test_cmdline_etheraddr.c | 8 +- app/test/test_net_ether.c | 165 ++++++++++++++++++++++++++++++ drivers/net/tap/rte_eth_tap.c | 25 +---- lib/net/rte_ether.c | 85 +++++++++++---- lib/net/rte_ether.h | 10 +- 6 files changed, 243 insertions(+), 51 deletions(-) create mode 100644 app/test/test_net_ether.c
Signature · 1 line
2.39.2
PATCH v3 1/4 test: remove some strings from cmdline_etheraddr tests
Some of the ethernet address formats which were invalid will now become valid inputs when rte_ether_unformat_addr is modified to allow leading zeros.
Also, make local variables static.
--- app/test/test_cmdline_etheraddr.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
app/test/test_cmdline_etheraddr.c +3 -5
@@ -20,7 +20,7 @@ struct ether_addr_str { };/* valid strings */ -const struct ether_addr_str ether_addr_valid_strs[] = { +static const struct ether_addr_str ether_addr_valid_strs[] = { {"01:23:45:67:89:AB" ,0xAB8967452301ULL }, {"4567:89AB:CDEF" ,0xEFCDAB896745ULL }, };
@@ -30,7 +30,7 @@ const struct ether_addr_str ether_addr_valid_strs[] = {* end of token, which is either space chars, null char or * a hash sign. */ -const char * ether_addr_garbage_strs[] = { +static const char *const ether_addr_garbage_strs[] = {"00:11:22:33:44:55\0garbage" ,"00:11:22:33:44:55#garbage" ,"00:11:22:33:44:55 garbage" ,
@@ -46,14 +46,13 @@ const char * ether_addr_garbage_strs[] = {#define GARBAGE_ETHERADDR 0x554433221100ULL/* corresponding address */ -const char * ether_addr_invalid_strs[] = { +static const char *const ether_addr_invalid_strs[] = {/* valid chars, invalid syntax */ "0123:45:67:89:AB" ,"01:23:4567:89:AB" ,"01:23:45:67:89AB" ,"012:345:678:9AB" ,"01:23:45:67:89:ABC" , -"01:23:45:67:89:A" ,"01:23:45:67:89" ,"01:23:45:67:89:AB:CD" ,/* invalid chars, valid syntax */
@@ -61,7 +60,6 @@ const char * ether_addr_invalid_strs[] = {"INVA:LIDC:HARS" ,/* misc */ "01 23 45 67 89 AB" , -"01.23.45.67.89.AB" ,"01,23,45,67,89,AB" ,"01:23:45\0:67:89:AB" , "01:23:45#:67:89:AB",
Signature · 1 line
2.39.2
PATCH v3 2/4 rte_ether_unformat: accept more inputs
In reply to Stephen HemmingerThis updates rte_ether_addr_unformat() to accept more types formats for MAC address. It allows IEEE, IETF and Cisco formats. Leading zeros are allowed for byte formats.
--- lib/net/rte_ether.c | 85 ++++++++++++++++++++++++++++++++++----------- lib/net/rte_ether.h | 10 ++++-- 2 files changed, 73 insertions(+), 22 deletions(-)
lib/net/rte_ether.c +65 -20
@@ -38,7 +38,8 @@ static int8_t get_xdigit(char ch) }/* Convert 00:11:22:33:44:55 to ethernet address */ -static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea) +static bool get_ether_addr6 (const char *s0,struct rte_ether_addr *ea, +const char sep) {const char *s = s0;int i;
@@ -48,25 +49,29 @@ static bool get_ether_addr6(const char *s0, struct rte_ether_addr *ea) x =get_xdigit (*s++);if (x <0 ) -return false; +return false;/* not a hex digit */ - ea->addr_bytes[i] = x <<4 ; - x =get_xdigit (*s++); -if (x <0 ) -return false; - ea->addr_bytes[i] |= x; + ea->addr_bytes[i] = x; +if (*s != sep && *s !='\0' ) { + x =get_xdigit (*s++); +if (x <0 ) +return false;/* not a hex digit */ + ea->addr_bytes[i] <<=4 ; + ea->addr_bytes[i] |= x; + }if (i <RTE_ETHER_ADDR_LEN -1 && - *s++ !=':' ) -return false; + *s++ != sep) +return false;/* premature end of string */ } -/* return true if at end of string */ +/* return true if no trailing characters */ return *s =='\0' ; }/* Convert 0011:2233:4455 to ethernet address */ -static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea) +static bool get_ether_addr3 (const char *s,struct rte_ether_addr *ea, +const char sep) {int i, j;
@@ -78,14 +83,15 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea) x =get_xdigit (*s++);if (x <0 ) -return false; +return false;/* not a hex digit */ w = (w <<4 ) | x; } + ea->addr_bytes[i] = w >>8 ; ea->addr_bytes[i +1 ] = w &0xff ;if (i <RTE_ETHER_ADDR_LEN -2 && - *s++ !=':' ) + *s++ != sep)return false; }
@@ -93,17 +99,56 @@ static bool get_ether_addr3(const char *s, struct rte_ether_addr *ea) }/* -* Like ether_aton_r but can handle either -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX -* and is more restrictive. +* Scan input to see if separated by dash, colon or period +* Returns separator and number of matches +* If separators are mixed will return +*/ +static unsigned int get_ether_sep (const char *s,char *sep) +{ +static const char separators[] ="-:." ; +unsigned int count =0 ; +const char *cp; + + cp =strpbrk (s, separators); +if (cp ==NULL ) +return 0 ;/* no separator found */ + + *sep = *cp;/* return the separator */ +do { + ++count; +/* find next instance of separator */ + cp =strchr (cp +1 , *sep); + }while (cp !=NULL ); + +return count; +} + +/* +* Be liberal in accepting a wide variety of notational formats +* for MAC address including: +* - Linux format six groups of hexadecimal digits separated by colon +* - Windows format six groups separated by hyphen +* - two groups hexadecimal digits */ int rte_ether_unformat_addr (const char *s,struct rte_ether_addr *ea) { -if (get_ether_addr6 (s, ea)) -return 0 ; -if (get_ether_addr3 (s, ea)) -return 0 ; +unsigned int count; +char sep ='\0' ; + + count =get_ether_sep (s, &sep); +switch (count) { +case 5 :/* i.e 01:23:45:67:89:AB */ +if (get_ether_addr6 (s, ea, sep)) +return 0 ; +break ; +case 2 :/* i.e 0123.4567.89AB */ +if (get_ether_addr3 (s, ea, sep)) +return 0 ; +break ; +default : +break ; + } rte_errno =EINVAL ;return -1 ;
lib/net/rte_ether.h +8 -2
@@ -254,9 +254,15 @@ rte_ether_format_addr(char *buf, uint16_t size,* * @param str * A pointer to buffer contains the formatted MAC address. -* The supported formats are: -* XX:XX:XX:XX:XX:XX or XXXX:XXXX:XXXX +* Accepts either byte or word format separated by colon, +* hyphen or period. +* +* The example formats are: +* XX:XX:XX:XX:XX:XX - Canonical form +* XX-XX-XX-XX-XX-XX - Windows and IEEE 802 +* XXXX.XXXX.XXXX - Cisco * where XX is a hex digit: 0-9, a-f, or A-F. +* In the byte format, leading zeros are optional. * @param eth_addr * A pointer to a ether_addr structure. * @return
Signature · 1 line
2.39.2
PATCH v3 3/4 test: add tests for rte_ether routines
In reply to Stephen HemmingerThis add some basic tests for rte_unformat_ether_addr and other functions in rte_ether.
--- app/test/meson.build | 1 + app/test/test_net_ether.c | 165 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 app/test/test_net_ether.c
app/test/meson.build +1 -0
@@ -124,6 +124,7 @@ source_file_deps = { 'test_meter.c': ['meter'], 'test_metrics.c': ['metrics'], 'test_mp_secondary.c': ['hash', 'lpm'], + 'test_net_ether.c': ['net'], 'test_pcapng.c': ['ethdev', 'net', 'pcapng'], 'test_pdcp.c': ['eventdev', 'pdcp', 'net', 'timer', 'security'], 'test_pdump.c': ['pdump'] + sample_packet_forward_deps,
app/test/test_net_ether.c new file +165 -0
165 lines not on this page · raw message
Signature · 1 line
2.39.2
PATCH v3 4/4 net/tap: use rte_ether_unformat_address
In reply to Stephen HemmingerFrom: David Christensen <[email protected]>
Building DPDK with gcc 12 on a ppc64le system generates a stringop-overflow warning. Replace the local MAC address validation function parse_user_mac() with a call to rte_ether_unformat_addr() instead.
Bugzilla ID: 1197
Cc: [email protected]
--- drivers/net/tap/rte_eth_tap.c | 25 +------------------------ 1 file changed, 1 insertion(+), 24 deletions(-)
drivers/net/tap/rte_eth_tap.c +1 -24
@@ -2267,29 +2267,6 @@ set_remote_iface(const char *key __rte_unused,return 0 ; } -static int parse_user_mac (struct rte_ether_addr *user_mac, -const char *value) -{ -unsigned int index =0 ; -char mac_temp[strlen (ETH_TAP_USR_MAC_FMT ) +1 ], *mac_byte =NULL ; - -if (user_mac ==NULL || value ==NULL ) -return 0 ; - -strlcpy (mac_temp, value,sizeof (mac_temp)); - mac_byte =strtok (mac_temp,":" ); - -while ((mac_byte !=NULL ) && - (strlen (mac_byte) <=2 ) && - (strlen (mac_byte) ==strspn (mac_byte, -ETH_TAP_CMP_MAC_FMT ))) { - user_mac->addr_bytes[index++] =strtoul (mac_byte,NULL ,16 ); - mac_byte =strtok (NULL ,":" ); - } - -return index; -} - static int set_mac_type(const char *key __rte_unused, const char *value,
@@ -2311,7 +2288,7 @@ set_mac_type(const char *key __rte_unused, goto success; } -if (parse_user_mac (user_mac, value) !=6 ) +if (rte_ether_unformat_addr (value, user_mac) <0 ) goto error; success:TAP_LOG (DEBUG ,"TAP user MAC param (%s)" , value);
Signature · 1 line
2.39.2
Re: [PATCH v3 4/4] net/tap: use rte_ether_unformat_address
Quoted 9 lines · Stephen Hemminger
From: David Christensen <[email protected]>
Building DPDK with gcc 12 on a ppc64le system generates a stringop-overflow warning. Replace the local MAC address validation function
parse_user_mac()with a call torte_ether_unformat_addr()instead.Bugzilla ID: 1197
For record, this should be 1195, caught by David:
Bugzilla ID: 1195
Re: [PATCH v3 0/4] rte_ether_unformat_addr related changes
In reply to Stephen HemmingerQuoted 10 lines · Stephen Hemminger
This patchset makes
rte_ether_unformat_addrallow other formats for MAC address. Need to remove some inputs from existingcmdline_etheraddrtest, and add a new test in test suite to cover this. There is some overlap between the two tests but that is fine.v4 - fix spelling errors
don't allow leading zeros in Cisco 3 part format incorporate tap patch to use unformat addr
Quoted 9 lines
David Christensen (1):
net/tap: userte_ether_unformat_addressStephen Hemminger (3):
test: remove some strings fromcmdline_etheraddrtests
rte_ether_unformat: accept more inputs
test: add tests forrte_etherroutines
Thanks for the patch, appreciated.
For series,
Series applied to dpdk-next-net/main, thanks.