On 20.03.2019 11:34, Ilya Maximets wrote:
> 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 <[email protected]>
>>>> ---
>>>
>>> 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
Also, note that this happens only for virtual interfaces like veth/tap because
kernel always tries to delay checksum calculation/validation as much as
possible.
Correct packets received from the wire will always have correct checksums.
>
>>
>>> 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);
>>>>
>>>>
>>
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
