Ian Campbell wrote:
> On Thu, 2011-10-27 at 06:28 +0100, Jim Fehlig wrote:
>
>> Jim Fehlig wrote:
>>
>>> Ian Campbell wrote:
>>>
>>>
>>>> On Wed, 2011-10-26 at 00:06 +0100, Jim Fehlig wrote:
>>>>
>>>>
>>>>
>>>>> I previously sent this from my @suse.com mail address without having
>>>>> subscribed it. Sending again now that I have done so...
>>>>>
>>>>> 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
>>>>>
>>>>>
>>>>>
>>>> Oh dear.
>>>>
>>>>
>>>>
>>>>
>>>>> 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?
>>>>>
>>>>>
>>>>>
>>>> I think overall your change is an improvement, some thoughts:
>>>>
>>>> For a tap device XENBUS_PATH is set in vif-common.sh:
>>>> elif [ "$type_if" = tap ]; then
>>>> # Check presence of compulsory args.
>>>> : ${INTERFACE:?}
>>>>
>>>> # Get xenbus_path from device name.
>>>> # The name is built like that: "tap${domid}.${devid}".
>>>> dev_=${dev#tap}
>>>> domid=${dev_%.*}
>>>> devid=${dev_#*.}
>>>>
>>>> XENBUS_PATH="/local/domain/0/backend/vif/$domid/$devid"
>>>> fi
>>>>
>>>> Could there be false positives from this?
>>>>
>>>>
>>> Hmm, yes, I think it is possible.
>>>
>>>
>> On second thought, maybe not. A false positive would mean two tap
>> devices with the same name right? AFAICT, that's not permitted.
>>
>
> Oh right, we are given $dev aren't we.
>
>>>
>>>
>>>> Perhaps we should be more
>>>> aggressively checking for the tapX.Y, where X and Y are integers, format
>>>> as well? (that's not foolproof either though).
>>>>
>>>>
>>>>
>>> Yeah, I don't think that buys us much.
>>>
>>>
>>>
>>>> Perhaps the toolstack could write something to xenstore containing the
>>>> literal tap device name which it asked qemu for? Then we can simply read
>>>> it back here, e.g. /libxl/tap/0/tapX.Y -> $XENBUS_PATH (0 being the
>>>> backend domain and the content being the xenbus path so we don't need to
>>>> magic it up).
>>>>
>>>>
>> The toolstack already writes something in xenstore, namely
>> $XENBUS_PATH/bridge.
>>
>
> XENBUS_PATH here is really the vif backend path, not the tap path,
> although they in some way are aliased so in many cases that ok. I was
> just thinking it might be useful to have a backend space for the tap
> device only (since the guest can see the vif backend dir).
>
So you prefer this approach to solving the problem?
>
>> IMO, the problem is in vif-bridge
>>
>> bridge=${bridge:-}
>> bridge=$(xenstore_read_default "$XENBUS_PATH/bridge" "$bridge")
>>
>> if [ -z "$bridge" ]
>> then
>> bridge=$(brctl show | cut -d "
>> " -f 2 | cut -f 1)
>>
>> if [ -z "$bridge" ]
>> then
>> fatal "Could not find bridge, and none was specified"
>> fi
>> else
>> ...
>>
>> If the toolstack hasn't written anything to xenstore, vif-bridge happily
>> connects the tap device to the first bridge it finds. Shouldn't
>> vif-bridge just exit if no bridge is specified?
>>
>
> I think that behaviour is historical (which isn't to say it's correct).
>
Connecting the device to an arbitrary bridge seems dangerous to me.
What if the bridge is on a sensitive VLAN?
> FWIW xl defaults to writing xenbr0. I don't know what xend does.
>
xend writes nothing to that node if bridge is not specified in the vif
config :-(. I suppose that is the reason for the hack in vif-bridge,
which was a bad fix IMO.
Thanks,
Jim
> Ian.
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home