On 20.03.2019 12:18, Ilya Maximets wrote: > 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
I meant CHECKSUM_UNNECESSARY. >> 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
