On 2/10/22 02:21, Tonghao Zhang wrote: > On Tue, Jan 25, 2022 at 3:16 PM Harold Huang <baymaxhu...@gmail.com> wrote: >> >> Looks good to me. >> >> Reviewed-by: Harold Huang <baymaxhu...@gmail.com> > Hi Ilya, do you have an opinion? > >> <xiangxia.m....@gmail.com> 于2022年1月25日周二 13:25写道： >>> >>> From: Tonghao Zhang <xiangxia.m....@gmail.com> >>> >>> The old version of openvswitch doesn't remove the padding from >>> packet before L3+ conntrack processing and then packets are dropped >>> in linux kernel stack. The patch [1] fixes the issue. We fix this >>> issue on gateway which running ovs-dpdk as a quick workaround. Padding >>> should be removed because tunnel size + inner size > 64B.
Hi. So, the sequence of events is following: 1. Packet arrives to the OVS userspace datapath, packet has padding. 2. OVS encapsulates it and sends to other host that runs OVS with kernel datapath. The kernel version is old and doesn't have fix [1], i.e kernel version is not 4.9.104+, 4.14.100+ or 4.16+. 3. Kernel datapath decapsulates the packet and sends to conntrack. 4. conntrack drops the packet as invalid, because it doesn't expect L2 padding. Is that correct? Some comments inline. >>> More detailes, see [1] >>> >>> [1] - >>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=9382fe71c0058465e942a633869629929102843d >>> Signed-off-by: Tonghao Zhang <xiangxia.m....@gmail.com> >>> --- >>> v2: add OVS_UNLIKELY >>> v1: this version was submitted a year ago >>> http://patchwork.ozlabs.org/project/openvswitch/patch/20201214030936.87354-1-xiangxia.m....@gmail.com/ >>> --- >>> lib/netdev-native-tnl.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c >>> index b89dfdd52a86..e23d71d4aec1 100644 >>> --- a/lib/netdev-native-tnl.c >>> +++ b/lib/netdev-native-tnl.c >>> @@ -149,11 +149,15 @@ void * >>> netdev_tnl_push_ip_header(struct dp_packet *packet, >>> const void *header, int size, int *ip_tot_size) >>> { >>> + int padding = dp_packet_l2_pad_size(packet); >>> struct eth_header *eth; >>> struct ip_header *ip; >>> struct ovs_16aligned_ip6_hdr *ip6; >>> >>> eth = dp_packet_push_uninit(packet, size); >>> + if (OVS_UNLIKELY(padding)) { Would be great to have a short comment here saying why we need to trim the packet. e.g. "Older kernels may drop the packet after decapsulation if it contains unexpected L2 padding." >>> + dp_packet_set_size(packet, dp_packet_size(packet) - padding); It doesn't seem intuitive to trim after we pushed the header. Can we do that before instead? >>> + } >>> *ip_tot_size = dp_packet_size(packet) - sizeof (struct eth_header); >>> >>> memcpy(eth, header, size); >>> -- >>> 2.27.0 >>> >>> _______________________________________________ >>> dev mailing list >>> d...@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
