18/12/2017 21:21, Adrien Mazarguil:
> On Mon, Dec 18, 2017 at 10:26:29AM -0800, Stephen Hemminger wrote:
> > On Mon, 18 Dec 2017 17:46:23 +0100
> > Adrien Mazarguil <adrien.mazarg...@6wind.com> wrote:
> >
> > > +static int
> > > +ether_addr_from_str(struct ether_addr *eth_addr, const char *str)
> > > +{
> > > + static const uint8_t conv[0x100] = {
> > > + ['0'] = 0x80, ['1'] = 0x81, ['2'] = 0x82, ['3'] = 0x83,
> > > + ['4'] = 0x84, ['5'] = 0x85, ['6'] = 0x86, ['7'] = 0x87,
> > > + ['8'] = 0x88, ['9'] = 0x89, ['a'] = 0x8a, ['b'] = 0x8b,
> > > + ['c'] = 0x8c, ['d'] = 0x8d, ['e'] = 0x8e, ['f'] = 0x8f,
> > > + ['A'] = 0x8a, ['B'] = 0x8b, ['C'] = 0x8c, ['D'] = 0x8d,
> > > + ['E'] = 0x8e, ['F'] = 0x8f, [':'] = 0x40, ['-'] = 0x40,
> > > + ['\0'] = 0x60,
> > > + };
> > > + uint64_t addr = 0;
> > > + uint64_t buf = 0;
> > > + unsigned int i = 0;
> > > + unsigned int n = 0;
> > > + uint8_t tmp;
> > > +
> > > + do {
> > > + tmp = conv[(int)*(str++)];
> > > + if (!tmp)
> > > + return -EINVAL;
> > > + if (tmp & 0x40) {
> > > + i += (i & 1) + (!i << 1);
> > > + addr = (addr << (i << 2)) | buf;
> > > + n += i;
> > > + buf = 0;
> > > + i = 0;
> > > + } else {
> > > + buf = (buf << 4) | (tmp & 0xf);
> > > + ++i;
> > > + }
> > > + } while (!(tmp & 0x20));
> > > + if (n > 12)
> > > + return -EINVAL;
> > > + i = RTE_DIM(eth_addr->addr_bytes);
> > > + while (i) {
> > > + eth_addr->addr_bytes[--i] = addr & 0xff;
> > > + addr >>= 8;
> > > + }
> > > + return 0;
> > > +}
> > > +
> >
> >
> > Why not ether_ntoa?
>
> Good question. For the following reasons:
>
> - I forgot about the existence of ether_ntoa() and didn't look it up seeing
> struct ether_addr is (re-)defined by rte_ether.h. What happens when one
> includes netinet/ether.h together with that file results in various
> conflicts that trigger a compilation error. This problem should be
> addressed first.
>
> - ether_ntoa() returns a static buffer and is not reentrant, ether_ntoa_r()
> is but as a GNU extension, I'm not sure it exists on other OSes. Even if
> this driver is currently targeted at Linux, this is likely not the case
> for other DPDK code relying on rte_ether.h.
>
> - I had ether_addr_from_str()'s code already ready and lying around for a
> future update in testpmd's flow command parser. No other MAC-48 conversion
> function I know of is as flexible as this version. The ability to omit ":"
> and entering partial addresses is a big plus IMO.
>
> I think both can coexist on their own merits. Since rte_ether.h needs to be
> fixed either way, how about I move this function in a separate commit and
> address the conflict with netinet/ether.h while there?
Looks to be a good plan.
