diff mbox series

[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces

Message ID 20191223213357.49351-2-eveliner@mellanox.com
State Superseded
Headers show
Series vswitchd: Allow setting MAC on DPDK interfaces | expand

Commit Message

Eveline Raine Dec. 23, 2019, 9:34 p.m. UTC
When setting mac address for an interface using ovs-vsctl command:

    ovs-vsctl set interface <iface> mac=XX:XX:XX:XX:XX:XX

iface_set_mac() is responsible to delegate a request to set MAC to a
netdev-specific set_etheraddr().

At the moment iface_set_mac() skips all interfaces except those with
type = "internal", making it impossible to change MAC on any DPDK port.
Since DPDK ports are owned by the OVS process, OVSDB can be considered
the source of truth for them. In particular, the source of truth for
their MAC addresses - so, OVS can take responsibility for setting them.

Therefore this check is extended to "dpdk" type.

Acked-by: Roni Bar Yanai <roniba@mellanox.com>
Tested-by: Adrian Chiris <adrianc@mellanox.com>
Signed-off-by: Eveline Raine <eveliner@mellanox.com>
---
 vswitchd/bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ilya Maximets Jan. 3, 2020, 2:56 p.m. UTC | #1
On 23.12.2019 22:34, Eveline Raine wrote:
> When setting mac address for an interface using ovs-vsctl command:
> 
>     ovs-vsctl set interface <iface> mac=XX:XX:XX:XX:XX:XX
> 
> iface_set_mac() is responsible to delegate a request to set MAC to a
> netdev-specific set_etheraddr().
> 
> At the moment iface_set_mac() skips all interfaces except those with
> type = "internal", making it impossible to change MAC on any DPDK port.
> Since DPDK ports are owned by the OVS process, OVSDB can be considered
> the source of truth for them. In particular, the source of truth for
> their MAC addresses - so, OVS can take responsibility for setting them.
> 
> Therefore this check is extended to "dpdk" type.
> 
> Acked-by: Roni Bar Yanai <roniba@mellanox.com>
> Tested-by: Adrian Chiris <adrianc@mellanox.com>
> Signed-off-by: Eveline Raine <eveliner@mellanox.com>
> ---
>  vswitchd/bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 5de0a264a..355364afd 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -4696,7 +4696,7 @@ iface_set_mac(const struct bridge *br, const struct port *port, struct iface *if
>      struct eth_addr ea, *mac = NULL;
>      struct iface *hw_addr_iface;
>  
> -    if (strcmp(iface->type, "internal")) {
> +    if (strcmp(iface->type, "internal") && strcmp(iface->type, "dpdk")) {

Hmm... Interesting.  I didn't look at this code closely before and it
actually allows setting mac only for internal ports.

Allowing only dpdk port types here doesn't seem right.  Why not allowing
all the port types?

The issue I can see in this code is that OVS inherits MACs for internal
ports from the physical ones, i.e. if we're changing the MAC of a physical
port we need to re-check all the internal ports and update their addresses.

This might happen almost automatically on linux since we probably will
receive if-notifier event about MAC changing and OVS will trigger re-applying
of the bridge configuration.  However, this will not happen in DPDK case.
Recently introduced manual if-notifier could help here, I guess.
But all of this needs to be tested.

Ben, do you see any other drawbacks that we should handle if we'll allow
changing MAC addresses for non-internal ports?  Or, maybe some issues with
my logic?

Best regards, Ilya Maximets.
Ben Pfaff Jan. 7, 2020, 12:25 a.m. UTC | #2
On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
> Ben, do you see any other drawbacks that we should handle if we'll allow
> changing MAC addresses for non-internal ports?  Or, maybe some issues with
> my logic?

It can cause surprises for interactions with regular system tools.
Anyone who uses "ip" or "ifconfig" to change the MAC will find it
changed back later (perhaps not immediately).  And if you un-set it in
the database, OVS doesn't know what to change it back to.

These drawbacks aren't there in the same way for devices that OVS "owns"
like internal devices or dpdk devices.  Well, I guess they are for OVS
internal devices to some extent, but for those OVS has some
responsibility to pick a reasonable MAC address to begin with.  If OVS
doesn't, then it causes confusion of its own through things like having
a machine's MAC address change if you create a bridge and move a
physical NIC onto it.  We had lots of experience with that early on with
OVS.
Ophir Munk Jan. 8, 2020, 10:48 a.m. UTC | #3
Hi,
There is an important use case for having OVS change MAC addresses of dpdk interfaces. 
OpenStack for example needs to update the MAC address of a VF assigned to a VM, where the corresponding VF representor is owned by dpdk. 
For some NIC vendors using "ifconfig" or "ip" commands - is not an option (if the NIC is not bifurcated). 
Therefore OpenStack should use the OVS API to set the MAC address for dpdk interfaces.
Along with Ben's explanation it seems right to only allow "internal" or "dpdk" port types to set the MAC.

Testing both patches [1] and [2] - passed successfully.
Acked-by: Ophir Munk <ophirmu@mellanox.com>

I hope patches [1] and [2] can be merged to master.

[1]
https://patchwork.ozlabs.org/patch/1186896/
("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")

[2]
https://patchwork.ozlabs.org/patch/1215075/
("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")

> -----Original Message-----
> From: Ben Pfaff <blp@ovn.org>
> Sent: Tuesday, January 7, 2020 2:26 AM
> To: Ilya Maximets <i.maximets@ovn.org>
> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
> Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>;
> Ophir Munk <ophirmu@mellanox.com>; Majd Dibbiny
> <majd@mellanox.com>; Roni Bar Yanai <roniba@mellanox.com>; Ameer
> Mahagneh <ameerm@mellanox.com>
> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
> 
> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
> > Ben, do you see any other drawbacks that we should handle if we'll
> > allow changing MAC addresses for non-internal ports?  Or, maybe some
> > issues with my logic?
> 
> It can cause surprises for interactions with regular system tools.
> Anyone who uses "ip" or "ifconfig" to change the MAC will find it changed
> back later (perhaps not immediately).  And if you un-set it in the database,
> OVS doesn't know what to change it back to.
> 
> These drawbacks aren't there in the same way for devices that OVS "owns"
> like internal devices or dpdk devices.  Well, I guess they are for OVS internal
> devices to some extent, but for those OVS has some responsibility to pick a
> reasonable MAC address to begin with.  If OVS doesn't, then it causes
> confusion of its own through things like having a machine's MAC address
> change if you create a bridge and move a physical NIC onto it.  We had lots of
> experience with that early on with OVS.
Ilya Maximets Jan. 9, 2020, 1:27 p.m. UTC | #4
On 08.01.2020 11:48, Ophir Munk wrote:
> Hi,
> There is an important use case for having OVS change MAC addresses of dpdk interfaces. 
> OpenStack for example needs to update the MAC address of a VF assigned to a VM, where the corresponding VF representor is owned by dpdk. 
> For some NIC vendors using "ifconfig" or "ip" commands - is not an option (if the NIC is not bifurcated). 
> Therefore OpenStack should use the OVS API to set the MAC address for dpdk interfaces.
> Along with Ben's explanation it seems right to only allow "internal" or "dpdk" port types to set the MAC.

There are still same issues for DPDK ports.  Just because OVS
doesn't necessarily "owns" them.  In case of Mellanox NICs, ports
are still could be configured by 'ip' tool due to bifurcated
drivers.  And this produces the same confusion.

> 
> Testing both patches [1] and [2] - passed successfully.
> Acked-by: Ophir Munk <ophirmu@mellanox.com>
> 
> I hope patches [1] and [2] can be merged to master.
> 
> [1]
> https://patchwork.ozlabs.org/patch/1186896/
> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
> 
> [2]
> https://patchwork.ozlabs.org/patch/1215075/
> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
> 
>> -----Original Message-----
>> From: Ben Pfaff <blp@ovn.org>
>> Sent: Tuesday, January 7, 2020 2:26 AM
>> To: Ilya Maximets <i.maximets@ovn.org>
>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
>> Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>;
>> Ophir Munk <ophirmu@mellanox.com>; Majd Dibbiny
>> <majd@mellanox.com>; Roni Bar Yanai <roniba@mellanox.com>; Ameer
>> Mahagneh <ameerm@mellanox.com>
>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>
>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>> Ben, do you see any other drawbacks that we should handle if we'll
>>> allow changing MAC addresses for non-internal ports?  Or, maybe some
>>> issues with my logic?
>>
>> It can cause surprises for interactions with regular system tools.
>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it changed
>> back later (perhaps not immediately).  And if you un-set it in the database,
>> OVS doesn't know what to change it back to.
>>
>> These drawbacks aren't there in the same way for devices that OVS "owns"
>> like internal devices or dpdk devices.  Well, I guess they are for OVS internal
>> devices to some extent, but for those OVS has some responsibility to pick a
>> reasonable MAC address to begin with.  If OVS doesn't, then it causes
>> confusion of its own through things like having a machine's MAC address
>> change if you create a bridge and move a physical NIC onto it.  We had lots of
>> experience with that early on with OVS.
Roni Bar Yanai Jan. 13, 2020, 11:23 a.m. UTC | #5
>-----Original Message-----
>From: Ilya Maximets <i.maximets@ovn.org>
>Sent: Thursday, January 9, 2020 3:28 PM
>To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>; Ilya
>Maximets <i.maximets@ovn.org>
>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe Levi
><moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>; Majd Dibbiny
><majd@mellanox.com>; Roni Bar Yanai <roniba@mellanox.com>; Ameer
>Mahagneh <ameerm@mellanox.com>
>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>
>On 08.01.2020 11:48, Ophir Munk wrote:
>> Hi,
>> There is an important use case for having OVS change MAC addresses of dpdk
>interfaces.
>> OpenStack for example needs to update the MAC address of a VF assigned to a
>VM, where the corresponding VF representor is owned by dpdk.
>> For some NIC vendors using "ifconfig" or "ip" commands - is not an option (if the
>NIC is not bifurcated).
>> Therefore OpenStack should use the OVS API to set the MAC address for dpdk
>interfaces.
>> Along with Ben's explanation it seems right to only allow "internal" or "dpdk"
>port types to set the MAC.
>
>There are still same issues for DPDK ports.  Just because OVS doesn't necessarily
>"owns" them.  In case of Mellanox NICs, ports are still could be configured by 'ip'
>tool due to bifurcated drivers.  And this produces the same confusion.
>
This makes it hard to integrate OVS-DPDK in cloud with port representor. There  
Is no way to configure MAC address for untrusted VM. If the driver is bifurcated 
there is some WA,  but this is not a generic solution and will not be used.
Any thoughts on how we can configure MAC for untrusted VM on such topologies?
>>
>> Testing both patches [1] and [2] - passed successfully.
>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
>>
>> I hope patches [1] and [2] can be merged to master.
>>
>> [1]
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>
>hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%40m
>ell
>>
>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d14
>9
>>
>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFABhYy
>Kw
>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>>
>> [2]
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>
>hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%40m
>ell
>>
>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d14
>9
>>
>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj9lqJ8
>t
>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>>
>>> -----Original Message-----
>>> From: Ben Pfaff <blp@ovn.org>
>>> Sent: Tuesday, January 7, 2020 2:26 AM
>>> To: Ilya Maximets <i.maximets@ovn.org>
>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
>>> Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>;
>>> Ophir Munk <ophirmu@mellanox.com>; Majd Dibbiny
><majd@mellanox.com>;
>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
>>> <ameerm@mellanox.com>
>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> interfaces
>>>
>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>>> Ben, do you see any other drawbacks that we should handle if we'll
>>>> allow changing MAC addresses for non-internal ports?  Or, maybe some
>>>> issues with my logic?
>>>
>>> It can cause surprises for interactions with regular system tools.
>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it
>>> changed back later (perhaps not immediately).  And if you un-set it
>>> in the database, OVS doesn't know what to change it back to.
>>>
>>> These drawbacks aren't there in the same way for devices that OVS "owns"
>>> like internal devices or dpdk devices.  Well, I guess they are for
>>> OVS internal devices to some extent, but for those OVS has some
>>> responsibility to pick a reasonable MAC address to begin with.  If
>>> OVS doesn't, then it causes confusion of its own through things like
>>> having a machine's MAC address change if you create a bridge and move
>>> a physical NIC onto it.  We had lots of experience with that early on with OVS.
Roni Bar Yanai Jan. 22, 2020, 2:49 p.m. UTC | #6
Hi Ilya, Ben

What is the plan to manage MAC addresses of DPDK ports with representors?
The solution should be generic and support non bifurcated drivers.
Currently we cannot integrate DPDK with port representors in platforms
 such as open stack.

Thanks,
Roni
>-----Original Message-----
>From: Roni Bar Yanai
>Sent: Monday, January 13, 2020 1:24 PM
>To: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
><ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>
>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe Levi
><moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>; Majd Dibbiny
><majd@mellanox.com>; Ameer Mahagneh <ameerm@mellanox.com>
>Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>
>
>
>>-----Original Message-----
>>From: Ilya Maximets <i.maximets@ovn.org>
>>Sent: Thursday, January 9, 2020 3:28 PM
>>To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>; Ilya
>>Maximets <i.maximets@ovn.org>
>>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
>>Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>; Majd
>>Dibbiny <majd@mellanox.com>; Roni Bar Yanai <roniba@mellanox.com>;
>>Ameer Mahagneh <ameerm@mellanox.com>
>>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>
>>On 08.01.2020 11:48, Ophir Munk wrote:
>>> Hi,
>>> There is an important use case for having OVS change MAC addresses of
>>> dpdk
>>interfaces.
>>> OpenStack for example needs to update the MAC address of a VF
>>> assigned to a
>>VM, where the corresponding VF representor is owned by dpdk.
>>> For some NIC vendors using "ifconfig" or "ip" commands - is not an
>>> option (if the
>>NIC is not bifurcated).
>>> Therefore OpenStack should use the OVS API to set the MAC address for
>>> dpdk
>>interfaces.
>>> Along with Ben's explanation it seems right to only allow "internal" or "dpdk"
>>port types to set the MAC.
>>
>>There are still same issues for DPDK ports.  Just because OVS doesn't
>>necessarily "owns" them.  In case of Mellanox NICs, ports are still could be
>configured by 'ip'
>>tool due to bifurcated drivers.  And this produces the same confusion.
>>
>This makes it hard to integrate OVS-DPDK in cloud with port representor. There Is
>no way to configure MAC address for untrusted VM. If the driver is bifurcated
>there is some WA,  but this is not a generic solution and will not be used.
>Any thoughts on how we can configure MAC for untrusted VM on such
>topologies?
>>>
>>> Testing both patches [1] and [2] - passed successfully.
>>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
>>>
>>> I hope patches [1] and [2] can be merged to master.
>>>
>>> [1]
>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>>> c
>>>
>>hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%40
>m
>>ell
>>>
>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d1
>4
>>9
>>>
>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFABhY
>y
>>Kw
>>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
>>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>>>
>>> [2]
>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>>> c
>>>
>>hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%40
>m
>>ell
>>>
>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d1
>4
>>9
>>>
>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj9lq
>J8
>>t
>>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
>>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>>>
>>>> -----Original Message-----
>>>> From: Ben Pfaff <blp@ovn.org>
>>>> Sent: Tuesday, January 7, 2020 2:26 AM
>>>> To: Ilya Maximets <i.maximets@ovn.org>
>>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>>> <adrianc@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Majd
>>>> Dibbiny
>><majd@mellanox.com>;
>>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
>>>> <ameerm@mellanox.com>
>>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>>> interfaces
>>>>
>>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>>>> Ben, do you see any other drawbacks that we should handle if we'll
>>>>> allow changing MAC addresses for non-internal ports?  Or, maybe
>>>>> some issues with my logic?
>>>>
>>>> It can cause surprises for interactions with regular system tools.
>>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it
>>>> changed back later (perhaps not immediately).  And if you un-set it
>>>> in the database, OVS doesn't know what to change it back to.
>>>>
>>>> These drawbacks aren't there in the same way for devices that OVS "owns"
>>>> like internal devices or dpdk devices.  Well, I guess they are for
>>>> OVS internal devices to some extent, but for those OVS has some
>>>> responsibility to pick a reasonable MAC address to begin with.  If
>>>> OVS doesn't, then it causes confusion of its own through things like
>>>> having a machine's MAC address change if you create a bridge and
>>>> move a physical NIC onto it.  We had lots of experience with that early on with
>OVS.
Ben Pfaff Jan. 23, 2020, 9:33 p.m. UTC | #7
The main problem is that the database is stateful.  I would not have the
same objection to an RPC to set an Ethernet address.  This could be
implemented via the ovs-appctl interface, if local-only is acceptable,
or via OpenFlow, if the controller needs to do it.

On Wed, Jan 22, 2020 at 02:49:47PM +0000, Roni Bar Yanai wrote:
> Hi Ilya, Ben
> 
> What is the plan to manage MAC addresses of DPDK ports with representors?
> The solution should be generic and support non bifurcated drivers.
> Currently we cannot integrate DPDK with port representors in platforms
>  such as open stack.
> 
> Thanks,
> Roni
> >-----Original Message-----
> >From: Roni Bar Yanai
> >Sent: Monday, January 13, 2020 1:24 PM
> >To: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
> ><ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>
> >Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe Levi
> ><moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>; Majd Dibbiny
> ><majd@mellanox.com>; Ameer Mahagneh <ameerm@mellanox.com>
> >Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
> >
> >
> >
> >>-----Original Message-----
> >>From: Ilya Maximets <i.maximets@ovn.org>
> >>Sent: Thursday, January 9, 2020 3:28 PM
> >>To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>; Ilya
> >>Maximets <i.maximets@ovn.org>
> >>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
> >>Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>; Majd
> >>Dibbiny <majd@mellanox.com>; Roni Bar Yanai <roniba@mellanox.com>;
> >>Ameer Mahagneh <ameerm@mellanox.com>
> >>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
> >>
> >>On 08.01.2020 11:48, Ophir Munk wrote:
> >>> Hi,
> >>> There is an important use case for having OVS change MAC addresses of
> >>> dpdk
> >>interfaces.
> >>> OpenStack for example needs to update the MAC address of a VF
> >>> assigned to a
> >>VM, where the corresponding VF representor is owned by dpdk.
> >>> For some NIC vendors using "ifconfig" or "ip" commands - is not an
> >>> option (if the
> >>NIC is not bifurcated).
> >>> Therefore OpenStack should use the OVS API to set the MAC address for
> >>> dpdk
> >>interfaces.
> >>> Along with Ben's explanation it seems right to only allow "internal" or "dpdk"
> >>port types to set the MAC.
> >>
> >>There are still same issues for DPDK ports.  Just because OVS doesn't
> >>necessarily "owns" them.  In case of Mellanox NICs, ports are still could be
> >configured by 'ip'
> >>tool due to bifurcated drivers.  And this produces the same confusion.
> >>
> >This makes it hard to integrate OVS-DPDK in cloud with port representor. There Is
> >no way to configure MAC address for untrusted VM. If the driver is bifurcated
> >there is some WA,  but this is not a generic solution and will not be used.
> >Any thoughts on how we can configure MAC for untrusted VM on such
> >topologies?
> >>>
> >>> Testing both patches [1] and [2] - passed successfully.
> >>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
> >>>
> >>> I hope patches [1] and [2] can be merged to master.
> >>>
> >>> [1]
> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >>> c
> >>>
> >>hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%40
> >m
> >>ell
> >>>
> >>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d1
> >4
> >>9
> >>>
> >>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFABhY
> >y
> >>Kw
> >>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
> >>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
> >>>
> >>> [2]
> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >>> c
> >>>
> >>hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%40
> >m
> >>ell
> >>>
> >>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d1
> >4
> >>9
> >>>
> >>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj9lq
> >J8
> >>t
> >>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
> >>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
> >>>
> >>>> -----Original Message-----
> >>>> From: Ben Pfaff <blp@ovn.org>
> >>>> Sent: Tuesday, January 7, 2020 2:26 AM
> >>>> To: Ilya Maximets <i.maximets@ovn.org>
> >>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
> >>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
> >>>> <adrianc@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Majd
> >>>> Dibbiny
> >><majd@mellanox.com>;
> >>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
> >>>> <ameerm@mellanox.com>
> >>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
> >>>> interfaces
> >>>>
> >>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
> >>>>> Ben, do you see any other drawbacks that we should handle if we'll
> >>>>> allow changing MAC addresses for non-internal ports?  Or, maybe
> >>>>> some issues with my logic?
> >>>>
> >>>> It can cause surprises for interactions with regular system tools.
> >>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it
> >>>> changed back later (perhaps not immediately).  And if you un-set it
> >>>> in the database, OVS doesn't know what to change it back to.
> >>>>
> >>>> These drawbacks aren't there in the same way for devices that OVS "owns"
> >>>> like internal devices or dpdk devices.  Well, I guess they are for
> >>>> OVS internal devices to some extent, but for those OVS has some
> >>>> responsibility to pick a reasonable MAC address to begin with.  If
> >>>> OVS doesn't, then it causes confusion of its own through things like
> >>>> having a machine's MAC address change if you create a bridge and
> >>>> move a physical NIC onto it.  We had lots of experience with that early on with
> >OVS.
Roni Bar Yanai Jan. 26, 2020, 7:47 a.m. UTC | #8
Thanks Ben.

>-----Original Message-----
>From: Ben Pfaff <blp@ovn.org>
>Sent: Thursday, January 23, 2020 11:33 PM
>To: Roni Bar Yanai <roniba@mellanox.com>
>Cc: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
><ophirmu@mellanox.com>; Eveline Raine <eveliner@mellanox.com>;
>dev@openvswitch.org; Moshe Levi <moshele@mellanox.com>; Adrian Chiris
><adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Ameer
>Mahagneh <ameerm@mellanox.com>
>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>
>The main problem is that the database is stateful.  I would not have the same
>objection to an RPC to set an Ethernet address.  This could be implemented via the
>ovs-appctl interface, if local-only is acceptable, or via OpenFlow, if the controller
>needs to do it.
>
>On Wed, Jan 22, 2020 at 02:49:47PM +0000, Roni Bar Yanai wrote:
>> Hi Ilya, Ben
>>
>> What is the plan to manage MAC addresses of DPDK ports with representors?
>> The solution should be generic and support non bifurcated drivers.
>> Currently we cannot integrate DPDK with port representors in platforms
>> such as open stack.
>>
>> Thanks,
>> Roni
>> >-----Original Message-----
>> >From: Roni Bar Yanai
>> >Sent: Monday, January 13, 2020 1:24 PM
>> >To: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
>> ><ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>
>> >Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org; Moshe
>> >Levi <moshele@mellanox.com>; Adrian Chiris <adrianc@mellanox.com>;
>> >Majd Dibbiny <majd@mellanox.com>; Ameer Mahagneh
>> ><ameerm@mellanox.com>
>> >Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>> >interfaces
>> >
>> >
>> >
>> >>-----Original Message-----
>> >>From: Ilya Maximets <i.maximets@ovn.org>
>> >>Sent: Thursday, January 9, 2020 3:28 PM
>> >>To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>; Ilya
>> >>Maximets <i.maximets@ovn.org>
>> >>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>> >>Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>> >><adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Roni Bar
>> >>Yanai <roniba@mellanox.com>; Ameer Mahagneh
><ameerm@mellanox.com>
>> >>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>> >>interfaces
>> >>
>> >>On 08.01.2020 11:48, Ophir Munk wrote:
>> >>> Hi,
>> >>> There is an important use case for having OVS change MAC addresses
>> >>> of dpdk
>> >>interfaces.
>> >>> OpenStack for example needs to update the MAC address of a VF
>> >>> assigned to a
>> >>VM, where the corresponding VF representor is owned by dpdk.
>> >>> For some NIC vendors using "ifconfig" or "ip" commands - is not an
>> >>> option (if the
>> >>NIC is not bifurcated).
>> >>> Therefore OpenStack should use the OVS API to set the MAC address
>> >>> for dpdk
>> >>interfaces.
>> >>> Along with Ben's explanation it seems right to only allow "internal" or
>"dpdk"
>> >>port types to set the MAC.
>> >>
>> >>There are still same issues for DPDK ports.  Just because OVS
>> >>doesn't necessarily "owns" them.  In case of Mellanox NICs, ports
>> >>are still could be
>> >configured by 'ip'
>> >>tool due to bifurcated drivers.  And this produces the same confusion.
>> >>
>> >This makes it hard to integrate OVS-DPDK in cloud with port
>> >representor. There Is no way to configure MAC address for untrusted
>> >VM. If the driver is bifurcated there is some WA,  but this is not a generic
>solution and will not be used.
>> >Any thoughts on how we can configure MAC for untrusted VM on such
>> >topologies?
>> >>>
>> >>> Testing both patches [1] and [2] - passed successfully.
>> >>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
>> >>>
>> >>> I hope patches [1] and [2] can be merged to master.
>> >>>
>> >>> [1]
>> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>> >>> pat
>> >>> c
>> >>>
>>
>>>hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%40
>> >m
>> >>ell
>> >>>
>>
>>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d
>1
>> >4
>> >>9
>> >>>
>>
>>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFABh
>Y
>> >y
>> >>Kw
>> >>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
>> >>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>> >>>
>> >>> [2]
>> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2F
>> >>> pat
>> >>> c
>> >>>
>>
>>>hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%40
>> >m
>> >>ell
>> >>>
>>
>>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4d
>1
>> >4
>> >>9
>> >>>
>>
>>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj9l
>q
>> >J8
>> >>t
>> >>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
>> >>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>> >>>
>> >>>> -----Original Message-----
>> >>>> From: Ben Pfaff <blp@ovn.org>
>> >>>> Sent: Tuesday, January 7, 2020 2:26 AM
>> >>>> To: Ilya Maximets <i.maximets@ovn.org>
>> >>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>> >>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>> >>>> <adrianc@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Majd
>> >>>> Dibbiny
>> >><majd@mellanox.com>;
>> >>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
>> >>>> <ameerm@mellanox.com>
>> >>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>> >>>> interfaces
>> >>>>
>> >>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>> >>>>> Ben, do you see any other drawbacks that we should handle if
>> >>>>> we'll allow changing MAC addresses for non-internal ports?  Or,
>> >>>>> maybe some issues with my logic?
>> >>>>
>> >>>> It can cause surprises for interactions with regular system tools.
>> >>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find it
>> >>>> changed back later (perhaps not immediately).  And if you un-set
>> >>>> it in the database, OVS doesn't know what to change it back to.
>> >>>>
>> >>>> These drawbacks aren't there in the same way for devices that OVS
>"owns"
>> >>>> like internal devices or dpdk devices.  Well, I guess they are
>> >>>> for OVS internal devices to some extent, but for those OVS has
>> >>>> some responsibility to pick a reasonable MAC address to begin
>> >>>> with.  If OVS doesn't, then it causes confusion of its own
>> >>>> through things like having a machine's MAC address change if you
>> >>>> create a bridge and move a physical NIC onto it.  We had lots of
>> >>>> experience with that early on with
>> >OVS.
Roni Bar Yanai April 20, 2020, 7:26 a.m. UTC | #9
Hi Ben, Ilya

Going back to this thread. We've tried app-ctl approach and it fails on consistency
problem. Orchestrator can configure on full system init, but now executing local 
restart will lose the configuration (again for bifurcated driver it is not problem).

The requirement for setting VF mac through the representor, comes from different
use cases such as Open Stack and Kubernetes. VF is used with pass-through and
untrusted user VM/Pod/Container . The MAC address is set by the orchestrator
only. For Linux use case there is ip tool for doing that, and Linux takes care of the
consistency. For OVS-DPDK with port representor, there is no way to configure VF MAC
address (except of for bifurcated drivers).

The requirement is to enable orchestrator configuration of VF MAC address in DPDK,
when working with port representor.
The solution should handle the generic use case and not bifurcated drivers only.
The MAC should be kept and configured in case of OVS restart. 

Maybe we can go back to the first solution and open set MAC to port representors
only. We treat the solution as a generic solution and ignore bifurcated drivers. 
When user configure the representor MAC (which is a reflection of the VF), the 
VF MAC is configured. The MAC address is saved in the DB. In case of returning back
to kernel there is no problem because the DB MAC applies only for DPDK representor.
For bifurcated driver, user can cause in consistency, but this is a unique case and 
we cannot defend against misuse of bifurcated drivers.

Any thoughts?

>-----Original Message-----
>From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Roni Bar Yanai
>Sent: Sunday, January 26, 2020 9:47 AM
>To: Ben Pfaff <blp@ovn.org>
>Cc: dev@openvswitch.org; Adrian Chiris <adrianc@mellanox.com>; Ilya Maximets
><i.maximets@ovn.org>; Ameer Mahagneh <ameerm@mellanox.com>; Eveline
>Raine <eveliner@mellanox.com>
>Subject: Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>interfaces
>
>Thanks Ben.
>
>>-----Original Message-----
>>From: Ben Pfaff <blp@ovn.org>
>>Sent: Thursday, January 23, 2020 11:33 PM
>>To: Roni Bar Yanai <roniba@mellanox.com>
>>Cc: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
>><ophirmu@mellanox.com>; Eveline Raine <eveliner@mellanox.com>;
>>dev@openvswitch.org; Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>><adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Ameer
>>Mahagneh <ameerm@mellanox.com>
>>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>
>>The main problem is that the database is stateful.  I would not have
>>the same objection to an RPC to set an Ethernet address.  This could be
>>implemented via the ovs-appctl interface, if local-only is acceptable,
>>or via OpenFlow, if the controller needs to do it.
>>
>>On Wed, Jan 22, 2020 at 02:49:47PM +0000, Roni Bar Yanai wrote:
>>> Hi Ilya, Ben
>>>
>>> What is the plan to manage MAC addresses of DPDK ports with representors?
>>> The solution should be generic and support non bifurcated drivers.
>>> Currently we cannot integrate DPDK with port representors in
>>> platforms such as open stack.
>>>
>>> Thanks,
>>> Roni
>>> >-----Original Message-----
>>> >From: Roni Bar Yanai
>>> >Sent: Monday, January 13, 2020 1:24 PM
>>> >To: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
>>> ><ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>
>>> >Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>> >Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>> ><adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Ameer
>>> >Mahagneh <ameerm@mellanox.com>
>>> >Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> >interfaces
>>> >
>>> >
>>> >
>>> >>-----Original Message-----
>>> >>From: Ilya Maximets <i.maximets@ovn.org>
>>> >>Sent: Thursday, January 9, 2020 3:28 PM
>>> >>To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>;
>>> >>Ilya Maximets <i.maximets@ovn.org>
>>> >>Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>> >>Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>> >><adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Roni Bar
>>> >>Yanai <roniba@mellanox.com>; Ameer Mahagneh
>><ameerm@mellanox.com>
>>> >>Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> >>interfaces
>>> >>
>>> >>On 08.01.2020 11:48, Ophir Munk wrote:
>>> >>> Hi,
>>> >>> There is an important use case for having OVS change MAC
>>> >>> addresses of dpdk
>>> >>interfaces.
>>> >>> OpenStack for example needs to update the MAC address of a VF
>>> >>> assigned to a
>>> >>VM, where the corresponding VF representor is owned by dpdk.
>>> >>> For some NIC vendors using "ifconfig" or "ip" commands - is not
>>> >>> an option (if the
>>> >>NIC is not bifurcated).
>>> >>> Therefore OpenStack should use the OVS API to set the MAC address
>>> >>> for dpdk
>>> >>interfaces.
>>> >>> Along with Ben's explanation it seems right to only allow
>>> >>> "internal" or
>>"dpdk"
>>> >>port types to set the MAC.
>>> >>
>>> >>There are still same issues for DPDK ports.  Just because OVS
>>> >>doesn't necessarily "owns" them.  In case of Mellanox NICs, ports
>>> >>are still could be
>>> >configured by 'ip'
>>> >>tool due to bifurcated drivers.  And this produces the same confusion.
>>> >>
>>> >This makes it hard to integrate OVS-DPDK in cloud with port
>>> >representor. There Is no way to configure MAC address for untrusted
>>> >VM. If the driver is bifurcated there is some WA,  but this is not a
>>> >generic
>>solution and will not be used.
>>> >Any thoughts on how we can configure MAC for untrusted VM on such
>>> >topologies?
>>> >>>
>>> >>> Testing both patches [1] and [2] - passed successfully.
>>> >>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
>>> >>>
>>> >>> I hope patches [1] and [2] can be merged to master.
>>> >>>
>>> >>> [1]
>>> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
>>> >>> F
>>> >>> pat
>>> >>> c
>>> >>>
>>>
>>>>hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%4
>0
>>> >m
>>> >>ell
>>> >>>
>>>
>>>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4
>d
>>1
>>> >4
>>> >>9
>>> >>>
>>>
>>>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFAB
>h
>>Y
>>> >y
>>> >>Kw
>>> >>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
>>> >>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>>> >>>
>>> >>> [2]
>>> >>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
>>> >>> F
>>> >>> pat
>>> >>> c
>>> >>>
>>>
>>>>hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%4
>0
>>> >m
>>> >>ell
>>> >>>
>>>
>>>>anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4
>d
>>1
>>> >4
>>> >>9
>>> >>>
>>>
>>>>256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj
>9l
>>q
>>> >J8
>>> >>t
>>> >>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
>>> >>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>>> >>>
>>> >>>> -----Original Message-----
>>> >>>> From: Ben Pfaff <blp@ovn.org>
>>> >>>> Sent: Tuesday, January 7, 2020 2:26 AM
>>> >>>> To: Ilya Maximets <i.maximets@ovn.org>
>>> >>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>> >>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>> >>>> <adrianc@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Majd
>>> >>>> Dibbiny
>>> >><majd@mellanox.com>;
>>> >>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
>>> >>>> <ameerm@mellanox.com>
>>> >>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>> >>>> interfaces
>>> >>>>
>>> >>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>> >>>>> Ben, do you see any other drawbacks that we should handle if
>>> >>>>> we'll allow changing MAC addresses for non-internal ports?  Or,
>>> >>>>> maybe some issues with my logic?
>>> >>>>
>>> >>>> It can cause surprises for interactions with regular system tools.
>>> >>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find
>>> >>>> it changed back later (perhaps not immediately).  And if you
>>> >>>> un-set it in the database, OVS doesn't know what to change it back to.
>>> >>>>
>>> >>>> These drawbacks aren't there in the same way for devices that
>>> >>>> OVS
>>"owns"
>>> >>>> like internal devices or dpdk devices.  Well, I guess they are
>>> >>>> for OVS internal devices to some extent, but for those OVS has
>>> >>>> some responsibility to pick a reasonable MAC address to begin
>>> >>>> with.  If OVS doesn't, then it causes confusion of its own
>>> >>>> through things like having a machine's MAC address change if you
>>> >>>> create a bridge and move a physical NIC onto it.  We had lots of
>>> >>>> experience with that early on with
>>> >OVS.
>_______________________________________________
>dev mailing list
>dev@openvswitch.org
>https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.open
>vswitch.org%2Fmailman%2Flistinfo%2Fovs-
>dev&amp;data=02%7C01%7Croniba%40mellanox.com%7C441d2f1d97634e35f1df0
>8d7a233f4d2%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63715621633
>9593360&amp;sdata=kIzF9FATNEowc5BxFQKDdkzQ8v%2BxpRy%2F2k%2BOHyGDT
>Eg%3D&amp;reserved=0
Ilya Maximets May 27, 2020, 3:49 p.m. UTC | #10
On 4/20/20 9:26 AM, Roni Bar Yanai wrote:
> Hi Ben, Ilya
> 
> Going back to this thread. We've tried app-ctl approach and it fails on consistency
> problem. Orchestrator can configure on full system init, but now executing local 
> restart will lose the configuration (again for bifurcated driver it is not problem).
> 
> The requirement for setting VF mac through the representor, comes from different
> use cases such as Open Stack and Kubernetes. VF is used with pass-through and
> untrusted user VM/Pod/Container . The MAC address is set by the orchestrator
> only. For Linux use case there is ip tool for doing that, and Linux takes care of the
> consistency. For OVS-DPDK with port representor, there is no way to configure VF MAC
> address (except of for bifurcated drivers).
> 
> The requirement is to enable orchestrator configuration of VF MAC address in DPDK,
> when working with port representor.
> The solution should handle the generic use case and not bifurcated drivers only.
> The MAC should be kept and configured in case of OVS restart. 
> 
> Maybe we can go back to the first solution and open set MAC to port representors
> only. We treat the solution as a generic solution and ignore bifurcated drivers. 
> When user configure the representor MAC (which is a reflection of the VF), the 
> VF MAC is configured. The MAC address is saved in the DB. In case of returning back
> to kernel there is no problem because the DB MAC applies only for DPDK representor.
> For bifurcated driver, user can cause in consistency, but this is a unique case and 
> we cannot defend against misuse of bifurcated drivers.
> 
> Any thoughts?

This is a DPDK specific issue.  Since appctl commnad is not suitable and common
'mac' configuration via database is a controversial solution, I'd suggest having
a scary-named netdev-dpdk specific knob to avoid abusing it for any other usecase.
For example, something like other_config:dpdk-vf-mac in the interface table.  And
the code should, probably, reject attempts to change mac address on non-representors.
Probably, the name of the knob should reflect that somehow.

What do you think?

One more note here: You mentioned untrusted VFs and containers. Last time I checked
DPDK sources I didn't see any "mac address administratively set" concept like it
done in kernel drivers, so the untrusted VM or container could easily change the
configured mac address. i.e. configuration from the OVS side is only part of the
solution that is required.

Best regards, Ilya Maximets.

> 
>> -----Original Message-----
>> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Roni Bar Yanai
>> Sent: Sunday, January 26, 2020 9:47 AM
>> To: Ben Pfaff <blp@ovn.org>
>> Cc: dev@openvswitch.org; Adrian Chiris <adrianc@mellanox.com>; Ilya Maximets
>> <i.maximets@ovn.org>; Ameer Mahagneh <ameerm@mellanox.com>; Eveline
>> Raine <eveliner@mellanox.com>
>> Subject: Re: [ovs-dev] [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>> interfaces
>>
>> Thanks Ben.
>>
>>> -----Original Message-----
>>> From: Ben Pfaff <blp@ovn.org>
>>> Sent: Thursday, January 23, 2020 11:33 PM
>>> To: Roni Bar Yanai <roniba@mellanox.com>
>>> Cc: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
>>> <ophirmu@mellanox.com>; Eveline Raine <eveliner@mellanox.com>;
>>> dev@openvswitch.org; Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>> <adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Ameer
>>> Mahagneh <ameerm@mellanox.com>
>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK interfaces
>>>
>>> The main problem is that the database is stateful.  I would not have
>>> the same objection to an RPC to set an Ethernet address.  This could be
>>> implemented via the ovs-appctl interface, if local-only is acceptable,
>>> or via OpenFlow, if the controller needs to do it.
>>>
>>> On Wed, Jan 22, 2020 at 02:49:47PM +0000, Roni Bar Yanai wrote:
>>>> Hi Ilya, Ben
>>>>
>>>> What is the plan to manage MAC addresses of DPDK ports with representors?
>>>> The solution should be generic and support non bifurcated drivers.
>>>> Currently we cannot integrate DPDK with port representors in
>>>> platforms such as open stack.
>>>>
>>>> Thanks,
>>>> Roni
>>>>> -----Original Message-----
>>>>> From: Roni Bar Yanai
>>>>> Sent: Monday, January 13, 2020 1:24 PM
>>>>> To: Ilya Maximets <i.maximets@ovn.org>; Ophir Munk
>>>>> <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>
>>>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>>>> <adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Ameer
>>>>> Mahagneh <ameerm@mellanox.com>
>>>>> Subject: RE: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>>>> interfaces
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ilya Maximets <i.maximets@ovn.org>
>>>>>> Sent: Thursday, January 9, 2020 3:28 PM
>>>>>> To: Ophir Munk <ophirmu@mellanox.com>; Ben Pfaff <blp@ovn.org>;
>>>>>> Ilya Maximets <i.maximets@ovn.org>
>>>>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>>>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>>>>> <adrianc@mellanox.com>; Majd Dibbiny <majd@mellanox.com>; Roni Bar
>>>>>> Yanai <roniba@mellanox.com>; Ameer Mahagneh
>>> <ameerm@mellanox.com>
>>>>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>>>>> interfaces
>>>>>>
>>>>>> On 08.01.2020 11:48, Ophir Munk wrote:
>>>>>>> Hi,
>>>>>>> There is an important use case for having OVS change MAC
>>>>>>> addresses of dpdk
>>>>>> interfaces.
>>>>>>> OpenStack for example needs to update the MAC address of a VF
>>>>>>> assigned to a
>>>>>> VM, where the corresponding VF representor is owned by dpdk.
>>>>>>> For some NIC vendors using "ifconfig" or "ip" commands - is not
>>>>>>> an option (if the
>>>>>> NIC is not bifurcated).
>>>>>>> Therefore OpenStack should use the OVS API to set the MAC address
>>>>>>> for dpdk
>>>>>> interfaces.
>>>>>>> Along with Ben's explanation it seems right to only allow
>>>>>>> "internal" or
>>> "dpdk"
>>>>>> port types to set the MAC.
>>>>>>
>>>>>> There are still same issues for DPDK ports.  Just because OVS
>>>>>> doesn't necessarily "owns" them.  In case of Mellanox NICs, ports
>>>>>> are still could be
>>>>> configured by 'ip'
>>>>>> tool due to bifurcated drivers.  And this produces the same confusion.
>>>>>>
>>>>> This makes it hard to integrate OVS-DPDK in cloud with port
>>>>> representor. There Is no way to configure MAC address for untrusted
>>>>> VM. If the driver is bifurcated there is some WA,  but this is not a
>>>>> generic
>>> solution and will not be used.
>>>>> Any thoughts on how we can configure MAC for untrusted VM on such
>>>>> topologies?
>>>>>>>
>>>>>>> Testing both patches [1] and [2] - passed successfully.
>>>>>>> Acked-by: Ophir Munk <ophirmu@mellanox.com>
>>>>>>>
>>>>>>> I hope patches [1] and [2] can be merged to master.
>>>>>>>
>>>>>>> [1]
>>>>>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
>>>>>>> F
>>>>>>> pat
>>>>>>> c
>>>>>>>
>>>>
>>>>> hwork.ozlabs.org%2Fpatch%2F1186896%2F&amp;data=02%7C01%7Croniba%4
>> 0
>>>>> m
>>>>>> ell
>>>>>>>
>>>>
>>>>> anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4
>> d
>>> 1
>>>>> 4
>>>>>> 9
>>>>>>>
>>>>
>>>>> 256f461b%7C0%7C0%7C637141732649459544&amp;sdata=okjLnR63gplGcbFAB
>> h
>>> Y
>>>>> y
>>>>>> Kw
>>>>>>> m0TXgT6VyGvlLtpnmRB%2Fk%3D&amp;reserved=0
>>>>>>> ("[ovs-dev,v2] netdev-dpdk: Add ability to set MAC address.")
>>>>>>>
>>>>>>> [2]
>>>>>>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2
>>>>>>> F
>>>>>>> pat
>>>>>>> c
>>>>>>>
>>>>
>>>>> hwork.ozlabs.org%2Fpatch%2F1215075%2F&amp;data=02%7C01%7Croniba%4
>> 0
>>>>> m
>>>>>> ell
>>>>>>>
>>>>
>>>>> anox.com%7Ca56df03e685a4f6ca39708d79507b4bf%7Ca652971c7d2e4d9ba6a4
>> d
>>> 1
>>>>> 4
>>>>>> 9
>>>>>>>
>>>>
>>>>> 256f461b%7C0%7C0%7C637141732649459544&amp;sdata=YovBQCaUkKBstJVj
>> 9l
>>> q
>>>>> J8
>>>>>> t
>>>>>>> 5aWo8ZHBvZzPZpFCzbUFM%3D&amp;reserved=0
>>>>>>> ("[ovs-dev,1/1] vswitchd: Allow setting MAC on DPDK interfaces")
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Ben Pfaff <blp@ovn.org>
>>>>>>>> Sent: Tuesday, January 7, 2020 2:26 AM
>>>>>>>> To: Ilya Maximets <i.maximets@ovn.org>
>>>>>>>> Cc: Eveline Raine <eveliner@mellanox.com>; dev@openvswitch.org;
>>>>>>>> Moshe Levi <moshele@mellanox.com>; Adrian Chiris
>>>>>>>> <adrianc@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Majd
>>>>>>>> Dibbiny
>>>>>> <majd@mellanox.com>;
>>>>>>>> Roni Bar Yanai <roniba@mellanox.com>; Ameer Mahagneh
>>>>>>>> <ameerm@mellanox.com>
>>>>>>>> Subject: Re: [PATCH 1/1] vswitchd: Allow setting MAC on DPDK
>>>>>>>> interfaces
>>>>>>>>
>>>>>>>> On Fri, Jan 03, 2020 at 03:56:59PM +0100, Ilya Maximets wrote:
>>>>>>>>> Ben, do you see any other drawbacks that we should handle if
>>>>>>>>> we'll allow changing MAC addresses for non-internal ports?  Or,
>>>>>>>>> maybe some issues with my logic?
>>>>>>>>
>>>>>>>> It can cause surprises for interactions with regular system tools.
>>>>>>>> Anyone who uses "ip" or "ifconfig" to change the MAC will find
>>>>>>>> it changed back later (perhaps not immediately).  And if you
>>>>>>>> un-set it in the database, OVS doesn't know what to change it back to.
>>>>>>>>
>>>>>>>> These drawbacks aren't there in the same way for devices that
>>>>>>>> OVS
>>> "owns"
>>>>>>>> like internal devices or dpdk devices.  Well, I guess they are
>>>>>>>> for OVS internal devices to some extent, but for those OVS has
>>>>>>>> some responsibility to pick a reasonable MAC address to begin
>>>>>>>> with.  If OVS doesn't, then it causes confusion of its own
>>>>>>>> through things like having a machine's MAC address change if you
>>>>>>>> create a bridge and move a physical NIC onto it.  We had lots of
>>>>>>>> experience with that early on with
>>>>> OVS.
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.open
>> vswitch.org%2Fmailman%2Flistinfo%2Fovs-
>> dev&amp;data=02%7C01%7Croniba%40mellanox.com%7C441d2f1d97634e35f1df0
>> 8d7a233f4d2%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63715621633
>> 9593360&amp;sdata=kIzF9FATNEowc5BxFQKDdkzQ8v%2BxpRy%2F2k%2BOHyGDT
>> Eg%3D&amp;reserved=0
Gaetan Rivet June 10, 2020, 11:34 a.m. UTC | #11
On 27/05/20 17:49 +0200, Ilya Maximets wrote:
> On 4/20/20 9:26 AM, Roni Bar Yanai wrote:
> > Hi Ben, Ilya
> > 
> > Going back to this thread. We've tried app-ctl approach and it fails on consistency
> > problem. Orchestrator can configure on full system init, but now executing local 
> > restart will lose the configuration (again for bifurcated driver it is not problem).
> > 
> > The requirement for setting VF mac through the representor, comes from different
> > use cases such as Open Stack and Kubernetes. VF is used with pass-through and
> > untrusted user VM/Pod/Container . The MAC address is set by the orchestrator
> > only. For Linux use case there is ip tool for doing that, and Linux takes care of the
> > consistency. For OVS-DPDK with port representor, there is no way to configure VF MAC
> > address (except of for bifurcated drivers).
> > 
> > The requirement is to enable orchestrator configuration of VF MAC address in DPDK,
> > when working with port representor.
> > The solution should handle the generic use case and not bifurcated drivers only.
> > The MAC should be kept and configured in case of OVS restart. 
> > 
> > Maybe we can go back to the first solution and open set MAC to port representors
> > only. We treat the solution as a generic solution and ignore bifurcated drivers. 
> > When user configure the representor MAC (which is a reflection of the VF), the 
> > VF MAC is configured. The MAC address is saved in the DB. In case of returning back
> > to kernel there is no problem because the DB MAC applies only for DPDK representor.
> > For bifurcated driver, user can cause in consistency, but this is a unique case and 
> > we cannot defend against misuse of bifurcated drivers.
> > 
> > Any thoughts?
> 
> This is a DPDK specific issue.  Since appctl commnad is not suitable and common
> 'mac' configuration via database is a controversial solution, I'd suggest having
> a scary-named netdev-dpdk specific knob to avoid abusing it for any other usecase.
> For example, something like other_config:dpdk-vf-mac in the interface table.  And
> the code should, probably, reject attempts to change mac address on non-representors.
> Probably, the name of the knob should reflect that somehow.
> 
> What do you think?

Hello Ilya,

I tried to read back the history about this subject but it's a little
scattered so apologies if I am repeating past comments.

I understand that you want to confine this issue to the specific case of
VF MAC.  Your solution addresses it but does not mitigate the split-brain
potential of bifurcated drivers.

The split-brain issue is inherent to those drivers.  I believe making a
feature more narrow and more difficult to find to side-step an intractable
problem will only muddy the water.

The confusion for bifurcated drivers is already there for other
interface states, such as MTU.  At this point wouldn't it be better to
be consistent in the risk involved, and expect users not to contradict
themselves when using their ports in OvS?

> 
> One more note here: You mentioned untrusted VFs and containers. Last time I checked
> DPDK sources I didn't see any "mac address administratively set" concept like it
> done in kernel drivers, so the untrusted VM or container could easily change the
> configured mac address. i.e. configuration from the OVS side is only part of the
> solution that is required.
> 
> Best regards, Ilya Maximets.
> 

I have looked at the VF specific configurations in DPDK, this concept is
currently addressed by vendor-specific APIs for a few drivers only.
I will propose a standard API for VF trusted mode (among others of those
VF APIs) to allow integration in upper layers, so that vendors can align
themselves.

Best regards,
diff mbox series

Patch

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 5de0a264a..355364afd 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -4696,7 +4696,7 @@  iface_set_mac(const struct bridge *br, const struct port *port, struct iface *if
     struct eth_addr ea, *mac = NULL;
     struct iface *hw_addr_iface;
 
-    if (strcmp(iface->type, "internal")) {
+    if (strcmp(iface->type, "internal") && strcmp(iface->type, "dpdk")) {
         return;
     }