[ovs-dev] netdev-dpdk: fix MAC address in port addr example

Message ID 5c31d1c4059c75b2b534f8661f13eb16317d3edc.1523294251.git.marcelo.leitner@gmail.com
State Accepted
Delegated to: Ian Stokes
Headers show
Series
  • [ovs-dev] netdev-dpdk: fix MAC address in port addr example
Related show

Commit Message

Marcelo Ricardo Leitner April 9, 2018, 5:20 p.m.
The MAC address is always 6-bytes long, never 7. The extra :01 and :02
doesn't belong in there as it doesn't mean selecting one port or
another.

Instead, use an incrementing MAC address, which is what usually happens
on such cards.

See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports sharing same PCI id")
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 Documentation/howto/dpdk.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

O Mahony, Billy April 11, 2018, 9:31 a.m. | #1
Hi Marcelo,

I haven't used the specific cards referred to in the surrounding documentation but I don't think the 'mac' address format is a typo.

The notation is specific to some vendor NICs that have several Ethernet devices sharing a single PCI bus:device.function address. In that case the PCI address alone cannot distinguish the Ethernet device to be uses ofr the dpdk port.

"Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address associated
with multiple ports. Using a PCI device like above won't work. Instead, below
usage is suggested::"

Regards,
Billy. 

> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of Marcelo Ricardo Leitner
> Sent: Monday, April 9, 2018 6:21 PM
> To: dev@openvswitch.org
> Cc: marcelo.leitner@gmail.com; slavash@mellanox.com
> Subject: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr example
> 
> The MAC address is always 6-bytes long, never 7. The extra :01 and :02 doesn't
> belong in there as it doesn't mean selecting one port or another.
> 
> Instead, use an incrementing MAC address, which is what usually happens on
> such cards.
> 
> See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
> Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports sharing same PCI
> id")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  Documentation/howto/dpdk.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> index
> 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979
> e07afa6f99cf903 100644
> --- a/Documentation/howto/dpdk.rst
> +++ b/Documentation/howto/dpdk.rst
> @@ -53,9 +53,9 @@ with multiple ports. Using a PCI device like above won't
> work. Instead, below  usage is suggested::
> 
>      $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
> +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
>      $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
> -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
> +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
> 
>  Note: such syntax won't support hotplug. The hotplug is supposed to work with
> future DPDK release, v18.05.
> --
> 2.14.3
> 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Marcelo Ricardo Leitner April 11, 2018, 12:10 p.m. | #2
Hi Billy,

On Wed, Apr 11, 2018 at 09:31:39AM +0000, O Mahony, Billy wrote:
> Hi Marcelo,
>
> I haven't used the specific cards referred to in the surrounding
> documentation but I don't think the 'mac' address format is a typo.
>
> The notation is specific to some vendor NICs that have several
> Ethernet devices sharing a single PCI bus:device.function address.
> In that case the PCI address alone cannot distinguish the Ethernet
> device to be uses ofr the dpdk port.

Yes, but

>
> "Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address associated
> with multiple ports. Using a PCI device like above won't work. Instead, below
> usage is suggested::"

I had initially reported this on dev@dpdk and Adrien confirmed that
the code is not prepared to handle the extra octet. (thread is
referenced in the See-also in the patch)

Also, even Mellanox is sending fixes to OpenStack to use standard MAC
addresses:
the bug: https://bugs.launchpad.net/os-net-config/+bug/1754256
part of the patch: https://review.openstack.org/#/c/550731/1/os_net_config/utils.py
which would be enough to specify port A or port B, as they have its
own MAC address.

Seems the doc just wasn't updated when they settled for a way of
addressing this issue.

Thanks,
Marcelo

>
> Regards,
> Billy.
>
> > -----Original Message-----
> > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > bounces@openvswitch.org] On Behalf Of Marcelo Ricardo Leitner
> > Sent: Monday, April 9, 2018 6:21 PM
> > To: dev@openvswitch.org
> > Cc: marcelo.leitner@gmail.com; slavash@mellanox.com
> > Subject: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr example
> >
> > The MAC address is always 6-bytes long, never 7. The extra :01 and :02 doesn't
> > belong in there as it doesn't mean selecting one port or another.
> >
> > Instead, use an incrementing MAC address, which is what usually happens on
> > such cards.
> >
> > See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
> > Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports sharing same PCI
> > id")
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> >  Documentation/howto/dpdk.rst | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
> > index
> > 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979
> > e07afa6f99cf903 100644
> > --- a/Documentation/howto/dpdk.rst
> > +++ b/Documentation/howto/dpdk.rst
> > @@ -53,9 +53,9 @@ with multiple ports. Using a PCI device like above won't
> > work. Instead, below  usage is suggested::
> >
> >      $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
> > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> >      $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
> > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
> > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
> >
> >  Note: such syntax won't support hotplug. The hotplug is supposed to work with
> > future DPDK release, v18.05.
> > --
> > 2.14.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian April 11, 2018, 12:15 p.m. | #3
> Hi Billy,
> 
> On Wed, Apr 11, 2018 at 09:31:39AM +0000, O Mahony, Billy wrote:
> > Hi Marcelo,
> >
> > I haven't used the specific cards referred to in the surrounding
> > documentation but I don't think the 'mac' address format is a typo.
> >
> > The notation is specific to some vendor NICs that have several
> > Ethernet devices sharing a single PCI bus:device.function address.
> > In that case the PCI address alone cannot distinguish the Ethernet
> > device to be uses ofr the dpdk port.
> 
> Yes, but
> 
> >
> > "Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address
> > associated with multiple ports. Using a PCI device like above won't
> > work. Instead, below usage is suggested::"
> 
> I had initially reported this on dev@dpdk and Adrien confirmed that the
> code is not prepared to handle the extra octet. (thread is referenced in
> the See-also in the patch)
> 
> Also, even Mellanox is sending fixes to OpenStack to use standard MAC
> addresses:
> the bug: https://bugs.launchpad.net/os-net-config/+bug/1754256
> part of the patch:
> https://review.openstack.org/#/c/550731/1/os_net_config/utils.py
> which would be enough to specify port A or port B, as they have its own
> MAC address.
> 
> Seems the doc just wasn't updated when they settled for a way of
> addressing this issue.

Agreed, I've validated with a Mellanox Connect X3 pro card, 6 bytes was expected.

Thanks for this Marcelo, I see Timothy Redaelli (from Red Hat also) has submitted a similar patch.

https://mail.openvswitch.org/pipermail/ovs-dev/2018-April/345885.html

There was a request to fix the commit message for that patch however.

@Timothy: As this patch has the correct commit message I can merge this to dpdk_merge if there are no objections rather than you spinning a v2 for yours?

Thanks
Ian

> 
> Thanks,
> Marcelo
> 
> >
> > Regards,
> > Billy.
> >
> > > -----Original Message-----
> > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > > bounces@openvswitch.org] On Behalf Of Marcelo Ricardo Leitner
> > > Sent: Monday, April 9, 2018 6:21 PM
> > > To: dev@openvswitch.org
> > > Cc: marcelo.leitner@gmail.com; slavash@mellanox.com
> > > Subject: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr
> > > example
> > >
> > > The MAC address is always 6-bytes long, never 7. The extra :01 and
> > > :02 doesn't belong in there as it doesn't mean selecting one port or
> another.
> > >
> > > Instead, use an incrementing MAC address, which is what usually
> > > happens on such cards.
> > >
> > > See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
> > > Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports
> > > sharing same PCI
> > > id")
> > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > ---
> > >  Documentation/howto/dpdk.rst | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/howto/dpdk.rst
> > > b/Documentation/howto/dpdk.rst index
> > > 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979
> > > e07afa6f99cf903 100644
> > > --- a/Documentation/howto/dpdk.rst
> > > +++ b/Documentation/howto/dpdk.rst
> > > @@ -53,9 +53,9 @@ with multiple ports. Using a PCI device like above
> > > won't work. Instead, below  usage is suggested::
> > >
> > >      $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0
> type=dpdk \
> > > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
> > > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> > >      $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1
> type=dpdk \
> > > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
> > > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
> > >
> > >  Note: such syntax won't support hotplug. The hotplug is supposed to
> > > work with future DPDK release, v18.05.
> > > --
> > > 2.14.3
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Marcelo Ricardo Leitner April 11, 2018, 12:25 p.m. | #4
On Wed, Apr 11, 2018 at 12:15:28PM +0000, Stokes, Ian wrote:
> > Seems the doc just wasn't updated when they settled for a way of
> > addressing this issue.
>
> Agreed, I've validated with a Mellanox Connect X3 pro card, 6 bytes
> was expected.
>
> Thanks for this Marcelo, I see Timothy Redaelli (from Red Hat also)
> has submitted a similar patch.
>
> https://mail.openvswitch.org/pipermail/ovs-dev/2018-April/345885.html

Oh, wasn't aware of that. Sorry Timothy, and thanks.

>
> There was a request to fix the commit message for that patch however.
>
> @Timothy: As this patch has the correct commit message I can merge
> this to dpdk_merge if there are no objections rather than you
> spinning a v2 for yours?
>
> Thanks
> Ian
>
> >
> > Thanks,
> > Marcelo
> >
> > >
> > > Regards,
> > > Billy.
> > >
> > > > -----Original Message-----
> > > > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > > > bounces@openvswitch.org] On Behalf Of Marcelo Ricardo Leitner
> > > > Sent: Monday, April 9, 2018 6:21 PM
> > > > To: dev@openvswitch.org
> > > > Cc: marcelo.leitner@gmail.com; slavash@mellanox.com
> > > > Subject: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr
> > > > example
> > > >
> > > > The MAC address is always 6-bytes long, never 7. The extra :01 and
> > > > :02 doesn't belong in there as it doesn't mean selecting one port or
> > another.
> > > >
> > > > Instead, use an incrementing MAC address, which is what usually
> > > > happens on such cards.
> > > >
> > > > See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
> > > > Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports
> > > > sharing same PCI
> > > > id")
> > > > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > > > ---
> > > >  Documentation/howto/dpdk.rst | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/Documentation/howto/dpdk.rst
> > > > b/Documentation/howto/dpdk.rst index
> > > > 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979
> > > > e07afa6f99cf903 100644
> > > > --- a/Documentation/howto/dpdk.rst
> > > > +++ b/Documentation/howto/dpdk.rst
> > > > @@ -53,9 +53,9 @@ with multiple ports. Using a PCI device like above
> > > > won't work. Instead, below  usage is suggested::
> > > >
> > > >      $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0
> > type=dpdk \
> > > > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
> > > > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> > > >      $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1
> > type=dpdk \
> > > > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
> > > > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
> > > >
> > > >  Note: such syntax won't support hotplug. The hotplug is supposed to
> > > > work with future DPDK release, v18.05.
> > > > --
> > > > 2.14.3
> > > >
> > > > _______________________________________________
> > > > dev mailing list
> > > > dev@openvswitch.org
> > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
O Mahony, Billy April 12, 2018, 11:54 a.m. | #5
Hi Marcelo,

Apologies. It wasn't clear that you had actually hands on experience of the issue.

Regards,
Billy. 

> -----Original Message-----
> From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> bounces@openvswitch.org] On Behalf Of O Mahony, Billy
> Sent: Wednesday, April 11, 2018 10:32 AM
> To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>;
> dev@openvswitch.org
> Cc: slavash@mellanox.com
> Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr
> example
> 
> Hi Marcelo,
> 
> I haven't used the specific cards referred to in the surrounding documentation
> but I don't think the 'mac' address format is a typo.
> 
> The notation is specific to some vendor NICs that have several Ethernet devices
> sharing a single PCI bus:device.function address. In that case the PCI address
> alone cannot distinguish the Ethernet device to be uses ofr the dpdk port.
> 
> "Some NICs (i.e. Mellanox ConnectX-3) have only one PCI address associated
> with multiple ports. Using a PCI device like above won't work. Instead, below
> usage is suggested::"
> 
> Regards,
> Billy.
> 
> > -----Original Message-----
> > From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-
> > bounces@openvswitch.org] On Behalf Of Marcelo Ricardo Leitner
> > Sent: Monday, April 9, 2018 6:21 PM
> > To: dev@openvswitch.org
> > Cc: marcelo.leitner@gmail.com; slavash@mellanox.com
> > Subject: [ovs-dev] [PATCH] netdev-dpdk: fix MAC address in port addr
> > example
> >
> > The MAC address is always 6-bytes long, never 7. The extra :01 and :02
> > doesn't belong in there as it doesn't mean selecting one port or another.
> >
> > Instead, use an incrementing MAC address, which is what usually
> > happens on such cards.
> >
> > See-also: http://www.dpdk.org/ml/archives/dev/2018-April/094976.html
> > Fixes: 5e7588186839 ("netdev-dpdk: fix port addition for ports sharing
> > same PCI
> > id")
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> > ---
> >  Documentation/howto/dpdk.rst | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/howto/dpdk.rst
> > b/Documentation/howto/dpdk.rst index
> >
> 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979
> > e07afa6f99cf903 100644
> > --- a/Documentation/howto/dpdk.rst
> > +++ b/Documentation/howto/dpdk.rst
> > @@ -53,9 +53,9 @@ with multiple ports. Using a PCI device like above
> > won't work. Instead, below  usage is suggested::
> >
> >      $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
> > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
> > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
> >      $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
> > -        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
> > +        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
> >
> >  Note: such syntax won't support hotplug. The hotplug is supposed to
> > work with future DPDK release, v18.05.
> > --
> > 2.14.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian April 17, 2018, 6:57 p.m. | #6
> On Wed, Apr 11, 2018 at 12:15:28PM +0000, Stokes, Ian wrote:
> > > Seems the doc just wasn't updated when they settled for a way of
> > > addressing this issue.
> >
> > Agreed, I've validated with a Mellanox Connect X3 pro card, 6 bytes
> > was expected.
> >
> > Thanks for this Marcelo, I see Timothy Redaelli (from Red Hat also)
> > has submitted a similar patch.
> >
> > https://mail.openvswitch.org/pipermail/ovs-dev/2018-April/345885.html
> 
> Oh, wasn't aware of that. Sorry Timothy, and thanks.

No problem, I've pushed this to DPDK_MERGE branch, will backport to ovs 2.9 also.

Thanks
Ian

Patch

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index 79b626c76d0dd45381bd75ab867b7815ca941208..69e692f40d500cf65d59d1979e07afa6f99cf903 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -53,9 +53,9 @@  with multiple ports. Using a PCI device like above won't work. Instead, below
 usage is suggested::
 
     $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \
-        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:01"
+        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55"
     $ ovs-vsctl add-port br0 dpdk-p1 -- set Interface dpdk-p1 type=dpdk \
-        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:55:02"
+        options:dpdk-devargs="class=eth,mac=00:11:22:33:44:56"
 
 Note: such syntax won't support hotplug. The hotplug is supposed to work with
 future DPDK release, v18.05.