diff mbox

[ovs-dev,3/3] rhel: Fix ifup and ifdown after DPDK naming change.

Message ID 20170125022153.28781-3-diproiettod@vmware.com
State Deferred
Headers show

Commit Message

Daniele Di Proietto Jan. 25, 2017, 2:21 a.m. UTC 
Names like dpdk0 and dpdk1 are not enough to identify a DPDK interface.
We could update README.RHEL.rst and add

OVS_EXTRA='set Interface ${DEVICE} options:dpdk-devargs=0000:01:00.0'

but a better solution is to add new parameters in the configuration file
to explicitly specify the dpdk-devargs.

Fixes: 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming")
Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
---
 rhel/README.RHEL.rst                        | 13 +++++++++----
 rhel/etc_sysconfig_network-scripts_ifup-ovs |  6 ++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

Comments

Ben Pfaff Feb. 2, 2017, 8:48 p.m. UTC | #1 
On Tue, Jan 24, 2017 at 06:21:53PM -0800, Daniele Di Proietto wrote:
> Names like dpdk0 and dpdk1 are not enough to identify a DPDK interface.
> We could update README.RHEL.rst and add
> 
> OVS_EXTRA='set Interface ${DEVICE} options:dpdk-devargs=0000:01:00.0'
> 
> but a better solution is to add new parameters in the configuration file
> to explicitly specify the dpdk-devargs.
> 
> Fixes: 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming")
> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>

This seems useful.

I don't understand why this uses "set" then $1.  Are you concerned that
BOND_DPDK_DEVARGS might have multiple words and you want to get just the
first one?

>  	OVSDPDKBond)
>  		ifup_ovs_bridge
> +		set -- ${BOND_DPDK_DEVARGS}
>  		for _iface in $BOND_IFACES; do
> -			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk"
> +			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk options:dpdk-devargs=$1"
> +			shift
>  		done
>  		ovs-vsctl -t ${TIMEOUT} \
>  			-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
Daniele Di Proietto Feb. 11, 2017, 12:14 a.m. UTC | #2 
On 02/02/2017 12:48, "Ben Pfaff" <blp@ovn.org> wrote:

>On Tue, Jan 24, 2017 at 06:21:53PM -0800, Daniele Di Proietto wrote:
>> Names like dpdk0 and dpdk1 are not enough to identify a DPDK interface.
>> We could update README.RHEL.rst and add
>> 
>> OVS_EXTRA='set Interface ${DEVICE} options:dpdk-devargs=0000:01:00.0'
>> 
>> but a better solution is to add new parameters in the configuration file
>> to explicitly specify the dpdk-devargs.
>> 
>> Fixes: 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming")
>> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
>
>This seems useful.

Hi Ben,

thanks for looking at this one and sorry for the delay.


>
>I don't understand why this uses "set" then $1.  Are you concerned that
>BOND_DPDK_DEVARGS might have multiple words and you want to get just the
>first one?

Now for each interface we need to specify two parameters: the name (it is
chosen by the user and it can be arbitrary) and the devargs (most likely the
PCI address).

With this patch the user enters the names in BOND_IFACES and the devargs in
BOND_DPDK_DEVARGS.

set -- ${BOND_DPDK_DEVARGS}
for _iface in ${BOND_IFACE}; do
    echo $_iface $1
    shift
done

is a quick and dirty way to iterate through both lists in the same loop.

Or maybe we could change the interface to specify in the same list the
name and the devargs.

Aaron, since you were looking at this as well, do you have any preference
on the user interface?

Thanks,

Daniele

>
>>  	OVSDPDKBond)
>>  		ifup_ovs_bridge
>> +		set -- ${BOND_DPDK_DEVARGS}
>>  		for _iface in $BOND_IFACES; do
>> -			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk"
>> +			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk options:dpdk-devargs=$1"
>> +			shift
>>  		done
>>  		ovs-vsctl -t ${TIMEOUT} \
>>  			-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
Ben Pfaff Feb. 11, 2017, 12:21 a.m. UTC | #3 
On Sat, Feb 11, 2017 at 12:14:24AM +0000, Daniele Di Proietto wrote:
> On 02/02/2017 12:48, "Ben Pfaff" <blp@ovn.org> wrote:
> >I don't understand why this uses "set" then $1.  Are you concerned that
> >BOND_DPDK_DEVARGS might have multiple words and you want to get just the
> >first one?
> 
> Now for each interface we need to specify two parameters: the name (it is
> chosen by the user and it can be arbitrary) and the devargs (most likely the
> PCI address).
> 
> With this patch the user enters the names in BOND_IFACES and the devargs in
> BOND_DPDK_DEVARGS.
> 
> set -- ${BOND_DPDK_DEVARGS}
> for _iface in ${BOND_IFACE}; do
>     echo $_iface $1
>     shift
> done
> 
> is a quick and dirty way to iterate through both lists in the same loop.

Oh!  I did not notice the new "shift".

Acked-by: Ben Pfaff <blp@ovn.org>
Aaron Conole Feb. 14, 2017, 5:39 p.m. UTC | #4 
Daniele Di Proietto <diproiettod@vmware.com> writes:

> On 02/02/2017 12:48, "Ben Pfaff" <blp@ovn.org> wrote:
>
>>On Tue, Jan 24, 2017 at 06:21:53PM -0800, Daniele Di Proietto wrote:
>>> Names like dpdk0 and dpdk1 are not enough to identify a DPDK interface.
>>> We could update README.RHEL.rst and add
>>> 
>>> OVS_EXTRA='set Interface ${DEVICE} options:dpdk-devargs=0000:01:00.0'
>>> 
>>> but a better solution is to add new parameters in the configuration file
>>> to explicitly specify the dpdk-devargs.
>>> 
>>> Fixes: 55e075e65ef9("netdev-dpdk: Arbitrary 'dpdk' port naming")
>>> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com>
>>
>>This seems useful.
>
> Hi Ben,
>
> thanks for looking at this one and sorry for the delay.
>
>
>>
>>I don't understand why this uses "set" then $1.  Are you concerned that
>>BOND_DPDK_DEVARGS might have multiple words and you want to get just the
>>first one?
>
> Now for each interface we need to specify two parameters: the name (it is
> chosen by the user and it can be arbitrary) and the devargs (most likely the
> PCI address).
>
> With this patch the user enters the names in BOND_IFACES and the devargs in
> BOND_DPDK_DEVARGS.
>
> set -- ${BOND_DPDK_DEVARGS}
> for _iface in ${BOND_IFACE}; do
>     echo $_iface $1
>     shift
> done
>
> is a quick and dirty way to iterate through both lists in the same loop.
>
> Or maybe we could change the interface to specify in the same list the
> name and the devargs.
>
> Aaron, since you were looking at this as well, do you have any preference
> on the user interface?

I don't have a preference, but I think typically put OVS_ in front of
variables in the ifcfg-* files.

Otherwise, ACK

> Thanks,
>
> Daniele
>
>>
>>>  	OVSDPDKBond)
>>>  		ifup_ovs_bridge
>>> +		set -- ${BOND_DPDK_DEVARGS}
>>>  		for _iface in $BOND_IFACES; do
>>> -			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk"
>>> +			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk options:dpdk-devargs=$1"
>>> +			shift
>>>  		done
>>>  		ovs-vsctl -t ${TIMEOUT} \
>>>  			-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
diff mbox

Patch

diff --git a/rhel/README.RHEL.rst b/rhel/README.RHEL.rst
index afccf1703..af4589325 100644
--- a/rhel/README.RHEL.rst
+++ b/rhel/README.RHEL.rst
@@ -266,14 +266,16 @@  DPDK NIC port:
 
 ::
 
-    ==> ifcfg-dpdk0 <==
-    DPDK vhost-user port:
-    DEVICE=dpdk0
+    ==> ifcfg-mydpdk0 <==
+    DEVICE=mydpdk0
+    DPDK_DEVARGS="0000:01:00.0"
     ONBOOT=yes
     DEVICETYPE=ovs
     TYPE=OVSDPDKPort
     OVS_BRIDGE=obr0
 
+DPDK vhost-user port:
+
 ::
 
     ==> ifcfg-vhu0 <==
@@ -283,6 +285,8 @@  DPDK NIC port:
     TYPE=OVSDPDKVhostUserPort
     OVS_BRIDGE=obr0
 
+DPDK bond:
+
 ::
 
     ==> ifcfg-bond0 <==
@@ -292,7 +296,8 @@  DPDK NIC port:
     TYPE=OVSDPDKBond
     OVS_BRIDGE=ovsbridge0
     BOOTPROTO=none
-    BOND_IFACES="dpdk0 dpdk1"
+    BOND_IFACES="mydpdk0 mydpdk1"
+    BOND_DPDK_DEVARGS="0000:01:00.0 0000:06:00.0"
     OVS_OPTIONS="bond_mode=active-backup"
     HOTPLUG=no
 
diff --git a/rhel/etc_sysconfig_network-scripts_ifup-ovs b/rhel/etc_sysconfig_network-scripts_ifup-ovs
index e49e6fe71..8fe60fcb1 100755
--- a/rhel/etc_sysconfig_network-scripts_ifup-ovs
+++ b/rhel/etc_sysconfig_network-scripts_ifup-ovs
@@ -170,7 +170,7 @@  case "$TYPE" in
 		ovs-vsctl -t ${TIMEOUT} \
 			-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \
 			-- add-port "$OVS_BRIDGE" "$DEVICE" $OVS_OPTIONS \
-			-- set Interface "$DEVICE" type=dpdk ${OVS_EXTRA+-- $OVS_EXTRA}
+			-- set Interface "$DEVICE" type=dpdk options:dpdk-devargs="${DPDK_DEVARGS}" ${OVS_EXTRA+-- $OVS_EXTRA}
 		;;
 	OVSDPDKRPort)
 		ifup_ovs_bridge
@@ -188,8 +188,10 @@  case "$TYPE" in
 		;;
 	OVSDPDKBond)
 		ifup_ovs_bridge
+		set -- ${BOND_DPDK_DEVARGS}
 		for _iface in $BOND_IFACES; do
-			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk"
+			IFACE_TYPES="${IFACE_TYPES} -- set interface ${_iface} type=dpdk options:dpdk-devargs=$1"
+			shift
 		done
 		ovs-vsctl -t ${TIMEOUT} \
 			-- --if-exists del-port "$OVS_BRIDGE" "$DEVICE" \