[ovs-dev] [ovs-dev, RFC] vswitchd: warn about misconfigured interface type on netdev bridge

Ilya Maximets i.maximets at samsung.com
Wed Mar 20 08:34:27 UTC 2019


On 20.03.2019 11:01, Eelco Chaudron wrote:
> 
> 
> On 19 Mar 2019, at 16:51, Ilya Maximets wrote:
> 
>> On 19.03.2019 12:23, Eelco Chaudron wrote:
>>> When debugging an issue we noticed that by accident someone has changes the
>>> bridge datapath_type to netdev, where it's clearly a kernel (system bridge)
>>> with physical and tap devices. Unfortunately, this is not something you
>>> will easily spot, as the bridge datapath type value is not shown by default.
>>>
>>> In addition, OVS is not warning you about this potential mismatch in
>>> interface and bridge datapath.
>>>
>>> I'm sending out this patch as an RFC as I could not find a clear
>>> demarcation between bridge datapath types and interface datapath types.
>>> The patch below will at least warn for netdev bridges with system
>>> interfaces. But no warning will be given for some unsupported virtual
>>> interfaces. For system bridges, the dpdk types will no be recognized as
>>> system/virtual interfaces (unless the name exists) which will result in
>>> an error.
>>>
>>> Signed-off-by: Eelco Chaudron <echaudro at redhat.com>
>>> ---
>>
>> Hi.
>>
>> Hmm. What do you mean under misconfiguration?
> 
> Good question, as the documentation and code are not clear about this part…
> 
> My interpretation is that for the netdev datapath bridges DPDK/vhost ports, internal ports, patch-ports, and user space tunnel ports are the once supported. But any other kernel ports are not, however, your text below suggests otherwise.

See below.

> 
>> In practice, userspace datapath is able to open "system" type netdevs.
>> It's supported by netdev-linux to open system network devices via socket.
>> And these devices has "system" type.
>> On 'datapath_type' changes, bridge/ofproto will be destroyed and re-created.
>> All the ports from kernel datapath will be destroyed and new ones created
>> in userspace. All the ports should still work as usual.
> 
> With the below statement in mind, I configure the following:
> 
>   ovs-vsctl add-br test
>   ip link add name right1 type veth peer name left1
>   ip link add name right2 type veth peer name left2
>   ovs-vsctl add-port test left1
>   ovs-vsctl add-port test left2
>   ovs-vsctl add-port test eth0
>   ip link set dev left1 up
>   ip link set dev left2 up
>   ip netns add netns1
>   ip netns add netns2
>   ip link set dev right1 netns netns1
>   ip link set dev right2 netns netns2
>   ip netns exec netns1 ip link set dev lo up
>   ip netns exec netns1 ip link set dev right1 up
>   ip netns exec netns2 ip link set dev right2 up
>   ip netns exec netns2 ip link set dev lo up
>   ip netns exec netns1 ip  a a dev right1 192.168.0.1/24
>   ip netns exec netns2 ip a a dev right2 192.168.0.2/24
> 
> This is all working fine now, and now some accidentally does this:
> 
>  ovs-vsctl set bridge test datapath_type=netdev
> 
> And you suggest all continues to work as is?

In general, yes.

> 
>> The only possible issue will be that this bridge will no longer communicate
>> with other bridges, because they're in different datapaths now.
>>
>> So, below warning will be printed on any attempt to add legitimate
>> system network interface to the userspace bridge.
> 
>> "system" devices supported by both datapaths, but "dpdk" devices supported
>> only by userspace, that is why you see the error in case of changing to
>> 'datapath_type=system'.
> 
> Are you saying ALL system devices are supported by the netdev datapath? Or only a specific set? If the later we should check for those (and maybe add some infrastructure to identify easily which devices are supported by which datapath. If it exists please point me to it, as I might have overlooked it).

OVS netdev datapath could open any linux network interfacce that could be
opened with the raw socket. There is nothing device specific here.

BTW, tests/system-userspace-testsuite.at completely relies on ability to
open and forward traffic through the veth pairs by netdev datapath.

> 
>> Maybe I'm missing something? What is the issue you're trying to address?
> 
> The above example stops working due to checksum offloading on veth.
> But if you are right this is a valid configuration I’ll further investigate this.

Configuration is valid. The issue here is that OVS netdev datapath doesn't
support TX checksum offloading (this is not easy task with arguable profit).
i.e. if packet arrives with bad/no checksum it will be sent to the output port
with same bad/no checksum. Everything works in case of kernel datapth because
the packet doesn't leave the kernel space. In case of netdev datapath some
information (like CHECKSUM_VALID skb flags) is lost while receiving via
socket in userspace and subsequently kernel expects valid checksum while
receiving the packet from userspace because TX offloading is not enabled.

This kind of issues usually mitigated by disabling TX offloading on the
"right*" interfaces, or by setting iptables to fill the checksums like this:

iptables -A POSTROUTING -t mangle -p udp -m udp -j CHECKSUM --checksum-fill

Some related OpenStack bug: https://bugs.launchpad.net/neutron/+bug/1244589

> 
>> Best regards, Ilya Maximets.
>>
>>>  vswitchd/bridge.c |    6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
>>> index a427b0122..42c33d1d9 100644
>>> --- a/vswitchd/bridge.c
>>> +++ b/vswitchd/bridge.c
>>> @@ -1808,6 +1808,12 @@ iface_do_create(const struct bridge *br,
>>>          goto error;
>>>      }
>>>
>>> +    if (!iface_is_internal(iface_cfg, br->cfg)
>>> +        && !strcmp(br->type, "netdev")
>>> +        && !strcmp(netdev_get_type(netdev), "system")) {
>>> +        VLOG_WARN("bridge %s: interface %s is a system type where the bridge "
>>> +                  "is a netdev one", br->name, iface_cfg->name);
>>> +    }
>>>      VLOG_INFO("bridge %s: added interface %s on port %d",
>>>                br->name, iface_cfg->name, *ofp_portp);
>>>
>>>
> 
> 


More information about the dev mailing list