Hi Flavio, Thank you for looking into this issue. It certainly improves the situation for broadcast frames when we have multiple tap ports on the bridge which are in DOWN state.
However, I see an issue with the patch. It does not handle device statistics. Earlier even when tap-port was DOWN, we would make a write system-call and kernel drops the packets which also accounts it in device statistics (drop counter). But now, since we skip sending packets to kernel then we will have to adjust it in netdev-linux. Can you modify the patch accordingly ? Warm Regards, Vishal Ajmera -----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Flavio Leitner Sent: Wednesday, January 10, 2018 9:37 PM To: [email protected] Cc: Flavio Leitner <[email protected]> Subject: [ovs-dev] [PATCH] netdev-linux: do not send packets to down tap ifaces. Today OVS pushes packets to the TAP interface ignoring its current state. That works because the kernel will return -EIO when it's not UP and OVS will just ignore that as it is not an OVS issue. However, it causes a huge impact when broadcasts happen when using userspace datapath accelerated with DPDK (e.g.: action NORMAL). This patch improves the situation by checking the TAP's interface state before issueing any syscall. However, there might be use-cases moving interfaces to other networking namespaces and in that case, OVS can't retrieve the iface state (sets it to DOWN). That would stop the traffic breaking the use-case. This patch relies on netlink notifications to find out if the device is local or not. When it's local, the device state is checked otherwise it will behave as before. Signed-off-by: Flavio Leitner <[email protected]> --- lib/netdev-linux.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index ccb6def6c..53b08bebd 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -502,6 +502,7 @@ struct netdev_linux { /* For devices of class netdev_tap_class only. */ int tap_fd; + bool present; /* If the device is present in the namespace */ }; struct netdev_rxq_linux { @@ -750,8 +751,10 @@ netdev_linux_update(struct netdev_linux *dev, dev->ifindex = change->if_index; dev->cache_valid |= VALID_IFINDEX; dev->get_ifindex_error = 0; + dev->present = true; } else { netdev_linux_changed(dev, change->ifi_flags, 0); + dev->present = false; } } else if (rtnetlink_type_is_rtnlgrp_addr(change->nlmsg_type)) { /* Invalidates in4, in6. */ @@ -1234,6 +1237,16 @@ netdev_linux_tap_batch_send(struct netdev *netdev_, { struct netdev_linux *netdev = netdev_linux_cast(netdev_); struct dp_packet *packet; + + /* The Linux tap driver returns EIO if the device is not up, + * so if the device is not up, don't waste time sending it. + * However, if the device is in another network namespace + * then OVS can't retrieve the state. In that case, send the + * packets anyway. */ + if (netdev->present && !(netdev->ifi_flags & IFF_UP)) { + return 0; + } + DP_PACKET_BATCH_FOR_EACH (packet, batch) { size_t size = dp_packet_size(packet); ssize_t retval; -- 2.14.3 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
