Prevent vif-bridge from adding user-created tap interfaces to a bridge

Prevent vif-bridge from adding user-created tap interfaces to a bridge

[Jim Fehlig]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

I received a report that vif-bridge adds any tap interface to a bridge,
regardless if xen is running and who created the tap interface.  E.g.

# tunctl -p -t tap42

will cause vif-bridge to be executed as per the following rule in
xen-backend.rules

SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"

I'm not sure how to improve the rule to prevent execution of vif-setup
in this case.  But it seems better to handle it in vif-bridge anyhow, by
not connecting the interface to a bridge if there is no corresponding
info in xenstore.  Something along the lines of the attached quick
patch.  Comments?

Thanks!
Jim


[-- Attachment #2: vif-bridge.patch --]
[-- Type: text/x-patch, Size: 945 bytes --]

# HG changeset patch
# User Jim Fehlig <jfehlig@novell.com>
# Date 1319581952 21600
# Node ID 74da2a3a1db1476d627f42e4a99e9e720cc6774d
# Parent  6c583d35d76dda2236c81d9437ff9d57ab02c006
Prevent vif-bridge from adding user-created tap interfaces to a bridge

Exit vif-bridge script if there is no device info in xenstore, preventing
it from adding user-created taps to bridges.

    Signed-off-by: Jim Fehlig <jfehlig@suse.com>

diff -r 6c583d35d76d -r 74da2a3a1db1 tools/hotplug/Linux/vif-bridge
--- a/tools/hotplug/Linux/vif-bridge	Thu Oct 20 15:36:01 2011 +0100
+++ b/tools/hotplug/Linux/vif-bridge	Tue Oct 25 16:32:32 2011 -0600
@@ -31,6 +31,13 @@
 
 dir=$(dirname "$0")
 . "$dir/vif-common.sh"
+
+domu=$(xenstore_read_default "$XENBUS_PATH/domain" "")
+if [ -z "$domu" ]
+then
+    log debug "No device details in $XENBUS_PATH, exiting."
+    exit 0
+fi
 
 bridge=${bridge:-}
 bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: Prevent vif-bridge from adding user-created tap interfaces to a bridge

[Ian Jackson]

Jim Fehlig writes ("[Xen-devel] Prevent vif-bridge from adding user-created tap interfaces to a bridge"):
> I received a report that vif-bridge adds any tap interface to a bridge,
> regardless if xen is running and who created the tap interface.  E.g.
> 
> # tunctl -p -t tap42
> 
> will cause vif-bridge to be executed as per the following rule in
> xen-backend.rules
> 
> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"

Urgh.  What a mess.

> I'm not sure how to improve the rule to prevent execution of vif-setup
> in this case.  But it seems better to handle it in vif-bridge anyhow, by
> not connecting the interface to a bridge if there is no corresponding
> info in xenstore.  Something along the lines of the attached quick
> patch.  Comments?

Aren't tap devices like this created by Xen's qemu ?  And as such we
should be letting qemu run the script, and not have any hotplug
script called by udev.

Ian.

Re: Prevent vif-bridge from adding user-created tap interfaces to a bridge

[Ian Campbell]

On Thu, 2011-10-27 at 16:12 +0100, Ian Jackson wrote:
> Jim Fehlig writes ("[Xen-devel] Prevent vif-bridge from adding user-created tap interfaces to a bridge"):
> > I received a report that vif-bridge adds any tap interface to a bridge,
> > regardless if xen is running and who created the tap interface.  E.g.
> > 
> > # tunctl -p -t tap42
> > 
> > will cause vif-bridge to be executed as per the following rule in
> > xen-backend.rules
> > 
> > SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
> > RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
> 
> Urgh.  What a mess.
> 
> > I'm not sure how to improve the rule to prevent execution of vif-setup
> > in this case.  But it seems better to handle it in vif-bridge anyhow, by
> > not connecting the interface to a bridge if there is no corresponding
> > info in xenstore.  Something along the lines of the attached quick
> > patch.  Comments?
> 
> Aren't tap devices like this created by Xen's qemu ?  And as such we
> should be letting qemu run the script, and not have any hotplug
> script called by udev.

We explicitly changed away from that scheme not so long ago. The issue
is that each tap has a vif counterpart which is somewhat logically the
same device and should be setup the same way, hence via the same
mechanisms.

Ian.

Re: Prevent vif-bridge from adding user-created tap interfaces to a bridge

[Jim Fehlig]

Ian Campbell wrote:
> On Thu, 2011-10-27 at 16:12 +0100, Ian Jackson wrote:
>   
>> Jim Fehlig writes ("[Xen-devel] Prevent vif-bridge from adding user-created tap interfaces to a bridge"):
>>     

Ok, my original post comes through now on a new thread...

>>> I received a report that vif-bridge adds any tap interface to a bridge,
>>> regardless if xen is running and who created the tap interface.  E.g.
>>>
>>> # tunctl -p -t tap42
>>>
>>> will cause vif-bridge to be executed as per the following rule in
>>> xen-backend.rules
>>>
>>> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>>> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>>       
>> Urgh.  What a mess.
>>
>>     
>>> I'm not sure how to improve the rule to prevent execution of vif-setup
>>> in this case.  But it seems better to handle it in vif-bridge anyhow, by
>>> not connecting the interface to a bridge if there is no corresponding
>>> info in xenstore.  Something along the lines of the attached quick
>>> patch.  Comments?
>>>       
>> Aren't tap devices like this created by Xen's qemu ?  And as such we
>> should be letting qemu run the script, and not have any hotplug
>> script called by udev.
>>     
>
> We explicitly changed away from that scheme not so long ago. The issue
> is that each tap has a vif counterpart which is somewhat logically the
> same device and should be setup the same way, hence via the same
> mechanisms.
>   

And qemu isn't involved when using netback.

So how to proceed?  Ian C. seemed to hesitantly ACK the patch in the
other thread [1] :).  The suggestion to write the info to another path
in xenstore can also be implemented, although IMO, that the path is not
accessible to the frontend would be the only benefit.

Thanks,
Jim

[1] http://lists.xensource.com/archives/html/xen-devel/2011-10/msg02016.html

Re: Prevent vif-bridge from adding user-created tap interfaces to a bridge

[Jim Fehlig]

Jim Fehlig wrote:
> Ian Campbell wrote:
>   
>> On Thu, 2011-10-27 at 16:12 +0100, Ian Jackson wrote:
>>   
>>     
>>> Jim Fehlig writes ("[Xen-devel] Prevent vif-bridge from adding user-created tap interfaces to a bridge"):
>>>     
>>>       
>
> Ok, my original post comes through now on a new thread...
>
>   
>>>> I received a report that vif-bridge adds any tap interface to a bridge,
>>>> regardless if xen is running and who created the tap interface.  E.g.
>>>>
>>>> # tunctl -p -t tap42
>>>>
>>>> will cause vif-bridge to be executed as per the following rule in
>>>> xen-backend.rules
>>>>
>>>> SUBSYSTEM=="net", KERNEL=="tap*", ACTION=="add",
>>>> RUN+="/etc/xen/scripts/vif-setup $env{ACTION} type_if=tap"
>>>>       
>>>>         
>>> Urgh.  What a mess.
>>>
>>>     
>>>       
>>>> I'm not sure how to improve the rule to prevent execution of vif-setup
>>>> in this case.  But it seems better to handle it in vif-bridge anyhow, by
>>>> not connecting the interface to a bridge if there is no corresponding
>>>> info in xenstore.  Something along the lines of the attached quick
>>>> patch.  Comments?
>>>>       
>>>>         
>>> Aren't tap devices like this created by Xen's qemu ?  And as such we
>>> should be letting qemu run the script, and not have any hotplug
>>> script called by udev.
>>>     
>>>       
>> We explicitly changed away from that scheme not so long ago. The issue
>> is that each tap has a vif counterpart which is somewhat logically the
>> same device and should be setup the same way, hence via the same
>> mechanisms.
>>   
>>     
>
> And qemu isn't involved when using netback.
>
> So how to proceed?  Ian C. seemed to hesitantly ACK the patch in the
> other thread [1] :).  The suggestion to write the info to another path
> in xenstore can also be implemented, although IMO, that the path is not
> accessible to the frontend would be the only benefit.
>   

Ping.

I'd like to add this patch to our downstream package but would like
upstream blessing first.

Thanks,
Jim

> Thanks,
> Jim
>
> [1] http://lists.xensource.com/archives/html/xen-devel/2011-10/msg02016.html
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>