On 14 June 2016 at 17:02, Brian Turek <[email protected]> wrote:
> I have no idea why my patch wouldn't be working, `git diff -C -M` is
> making the diff for me (if the below doesn't work again, I'll have to
> investigate).
>
From this email, if you had to apply the patch on a clean openvswitch
master branch cloned on a linux host, how would you apply?
>
> Regardless, I really would like to have the tag option be a part of the
> initial add-port call instead of an additional set-port call. If malformed
> input is a concern, how about something like:
>
> diff --git a/utilities/ovs-docker b/utilities/ovs-docker
> index 43cea54..edd48d2 100755
> --- a/utilities/ovs-docker
> +++ b/utilities/ovs-docker
> @@ -90,6 +90,16 @@ add_port () {
> MTU=`expr X"$1" : 'X[^=]*=\(.*\)'`
> shift
> ;;
> + --vlan=*)
> + if [ "$1" -eq "$1" ] 2>/dev/null
>
The above will always thrown an error as you are likely testing [
"--vlan=2" -eq "--vlan=2" ]. But I think I know what you are trying to do
and it should be okay. Please test and resend via "git send-email" with a
Signed-off-by.
+ then
> + TAG="tag=$(expr X"$1" : 'X[^=]*=\(.*\)')"
>
+ else
> + echo >&2 "$UTIL add-port: vlan must be an integer"
> + exit 1
> + fi
> + shift
> + ;;
> *)
> echo >&2 "$UTIL add-port: unknown option \"$1\""
> exit 1
> @@ -126,7 +136,7 @@ add_port () {
>
> # Add one end of veth to OVS bridge.
> if ovs_vsctl --may-exist add-port "$BRIDGE" "${PORTNAME}_l" \
> - -- set interface "${PORTNAME}_l" \
> + $TAG -- set interface "${PORTNAME}_l" \
> external_ids:container_id="$CONTAINER" \
> external_ids:container_iface="$INTERFACE"; then :; else
> echo >&2 "$UTIL: Failed to add "${PORTNAME}_l" port to bridge
> $BRIDGE"
> @@ -224,15 +234,15 @@ usage: ${UTIL} COMMAND
> Commands:
> add-port BRIDGE INTERFACE CONTAINER [--ipaddress="ADDRESS"]
> [--gateway=GATEWAY] [--macaddress="MACADDRESS"]
> - [--mtu=MTU]
> + [--mtu=MTU] [--vlan=TAG]
> Adds INTERFACE inside CONTAINER and connects it as a
> port
> in Open vSwitch BRIDGE. Optionally, sets ADDRESS on
> INTERFACE. ADDRESS can include a '/' to represent
> network
> - prefix length. Optionally, sets a GATEWAY, MACADDRESS
> - and MTU. e.g.:
> + prefix length. Optionally, sets a GATEWAY, MACADDRESS,
> + MTU, and VLAN. e.g.:
> ${UTIL} add-port br-int eth1 c474a0e2830e
> --ipaddress=192.168.1.2/24 --gateway=192.168.1.1
> - --macaddress="a2:c3:0d:49:7f:f8" --mtu=1450
> + --macaddress="a2:c3:0d:49:7f:f8" --mtu=1450 --vlan=10
> del-port BRIDGE INTERFACE CONTAINER
> Deletes INTERFACE inside CONTAINER and removes its
> connection to Open vSwitch BRIDGE. e.g.:
>
> On Mon, Jun 13, 2016 at 11:25 AM, Guru Shetty <[email protected]> wrote:
>
>>
>>
>> On 11 June 2016 at 05:28, Brian Turek <[email protected]> wrote:
>>
>>> Signed-off-by: Brian Turek <[email protected]>
>>>
>>
>> Thank you for your contribution. How did you send this patch? I couldn't
>> apply it on my local tree using 'git am'. Sending it via 'git send-email'
>> usually gets it right.
>>
>> The change in the patch itself is not much, so I took a look. The thing
>> that concerns me a bit with this patch is that I could potentially run
>> commands of the form:
>>
>> utilities/ovs-docker add-port br-int eth1 f5f314682193 --vlan="2 -- set
>> bridge br-int external_ids:hello=hi"
>>
>> We could avoid it by doing something like this instead:
>> diff --git a/utilities/ovs-docker b/utilities/ovs-docker
>> index 43cea54..82c0343 100755
>> --- a/utilities/ovs-docker
>> +++ b/utilities/ovs-docker
>> @@ -90,6 +90,10 @@ add_port () {
>> MTU=`expr X"$1" : 'X[^=]*=\(.*\)'`
>> shift
>> ;;
>> + --vlan=*)
>> + TAG=`expr X"$1" : 'X[^=]*=\(.*\)'`
>> + shift
>> + ;;
>> *)
>> echo >&2 "$UTIL add-port: unknown option \"$1\""
>> exit 1
>> @@ -134,6 +138,15 @@ add_port () {
>> exit 1
>> fi
>>
>> + if [ -n "$TAG" ]; then
>> + if ovs_vsctl set port "${PORTNAME}_l" tag="$TAG"; then :; else
>> + echo >&2 "$UTIL: Failed to set vlan $tag"
>> + ovs_vsctl del-port "${PORTNAME}_l"
>> + ip link delete "${PORTNAME}_l"
>> + exit 1
>> + fi
>> + fi
>> +
>> ip link set "${PORTNAME}_l" up
>>
>>
>>
>>
>>
>>>
>>> diff --git a/utilities/ovs-docker b/utilities/ovs-docker
>>> index 43cea54..98892b6 100755
>>> --- a/utilities/ovs-docker
>>> +++ b/utilities/ovs-docker
>>> @@ -90,6 +90,10 @@ add_port () {
>>> MTU=`expr X"$1" : 'X[^=]*=\(.*\)'`
>>> shift
>>> ;;
>>> + --vlan=*)
>>> + TAG="tag=$(expr X"$1" : 'X[^=]*=\(.*\)')"
>>> + shift
>>> + ;;
>>> *)
>>> echo >&2 "$UTIL add-port: unknown option \"$1\""
>>> exit 1
>>> @@ -126,7 +130,7 @@ add_port () {
>>>
>>> # Add one end of veth to OVS bridge.
>>> if ovs_vsctl --may-exist add-port "$BRIDGE" "${PORTNAME}_l" \
>>> - -- set interface "${PORTNAME}_l" \
>>> + $TAG -- set interface "${PORTNAME}_l" \
>>> external_ids:container_id="$CONTAINER" \
>>> external_ids:container_iface="$INTERFACE"; then :; else
>>> echo >&2 "$UTIL: Failed to add "${PORTNAME}_l" port to bridge
>>> $BRIDGE"
>>> @@ -224,15 +228,15 @@ usage: ${UTIL} COMMAND
>>> Commands:
>>> add-port BRIDGE INTERFACE CONTAINER [--ipaddress="ADDRESS"]
>>> [--gateway=GATEWAY] [--macaddress="MACADDRESS"]
>>> - [--mtu=MTU]
>>> + [--mtu=MTU] [--vlan=VLAN]
>>> Adds INTERFACE inside CONTAINER and connects it as a
>>> port
>>> in Open vSwitch BRIDGE. Optionally, sets ADDRESS on
>>> INTERFACE. ADDRESS can include a '/' to represent
>>> network
>>> - prefix length. Optionally, sets a GATEWAY,
>>> MACADDRESS
>>> - and MTU. e.g.:
>>> + prefix length. Optionally, sets a GATEWAY,
>>> MACADDRESS,
>>> + MTU, and VLAN. e.g.:
>>> ${UTIL} add-port br-int eth1 c474a0e2830e
>>> --ipaddress=192.168.1.2/24 --gateway=192.168.1.1
>>> - --macaddress="a2:c3:0d:49:7f:f8" --mtu=1450
>>> + --macaddress="a2:c3:0d:49:7f:f8" --mtu=1450
>>> --vlan=10
>>> del-port BRIDGE INTERFACE CONTAINER
>>> Deletes INTERFACE inside CONTAINER and removes its
>>> connection to Open vSwitch BRIDGE. e.g.:
>>> _______________________________________________
>>> dev mailing list
>>> [email protected]
>>> http://openvswitch.org/mailman/listinfo/dev
>>>
>>
>>
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
