| Message ID | 20240802085222.30784-1-sunyang.wu@jaguarmicro.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev] tunnel: Fix the bug in calculating the pop header length. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | fail | test: fail |
On 8/2/24 10:52, Sunyang Wu via dev wrote: > In the original logic, when the Layer 3 protocol is IPv4, hlen equals > sizeof(struct eth_header) + IP_HEADER_LEN. For IPv6, hlen equals > sizeof(struct eth_header) + packet->l4_ofs - packet->l3_ofs. However, in > the VLAN over VXLAN scenario, the hlen length does not include the VLAN > length, leading to errors in popping the header. Hi, Sunyang Wu. Thanks for the patch! Could you, please, explain your setup a little bit more? How do you end up in the tunnel pop while having a VLAN header in the outer packet? My initial thought is that you need to pop the vlan first before decapsulating VXLAN, otherwise you may end up with inconsistent packet metadata, i.e. OVS may still think that that packet has VLAN header while it was actually removed during decapsulation. We may need to add a check and fail decapsulation explicitly instead. Best regards, Ilya Maximets. > Now, hlen is uniformly > modified to packet->l4_ofs, which includes the total length of both > Layer 2 and Layer 3 headers, thereby correctly removing the tunnel > header. > > Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com> > --- > lib/netdev-native-tnl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c > index 16c56608d..1db258daf 100644 > --- a/lib/netdev-native-tnl.c > +++ b/lib/netdev-native-tnl.c > @@ -117,7 +117,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl, > tnl->ip_tos = ip->ip_tos; > tnl->ip_ttl = ip->ip_ttl; > > - *hlen += IP_HEADER_LEN; > + *hlen += packet->l4_ofs - sizeof(struct eth_header); > > } else if (IP_VER(ip->ip_ihl_ver) == 6) { > ovs_be32 tc_flow = get_16aligned_be32(&ip6->ip6_flow); > @@ -128,7 +128,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl, > tnl->ip_tos = ntohl(tc_flow) >> 20; > tnl->ip_ttl = ip6->ip6_hlim; > > - *hlen += packet->l4_ofs - packet->l3_ofs; > + *hlen += packet->l4_ofs - sizeof(struct eth_header); > > } else { > VLOG_WARN_RL(&err_rl, "ipv4 packet has invalid version (%d)",
diff --git a/lib/netdev-native-tnl.c b/lib/netdev-native-tnl.c index 16c56608d..1db258daf 100644 --- a/lib/netdev-native-tnl.c +++ b/lib/netdev-native-tnl.c @@ -117,7 +117,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl, tnl->ip_tos = ip->ip_tos; tnl->ip_ttl = ip->ip_ttl; - *hlen += IP_HEADER_LEN; + *hlen += packet->l4_ofs - sizeof(struct eth_header); } else if (IP_VER(ip->ip_ihl_ver) == 6) { ovs_be32 tc_flow = get_16aligned_be32(&ip6->ip6_flow); @@ -128,7 +128,7 @@ netdev_tnl_ip_extract_tnl_md(struct dp_packet *packet, struct flow_tnl *tnl, tnl->ip_tos = ntohl(tc_flow) >> 20; tnl->ip_ttl = ip6->ip6_hlim; - *hlen += packet->l4_ofs - packet->l3_ofs; + *hlen += packet->l4_ofs - sizeof(struct eth_header); } else { VLOG_WARN_RL(&err_rl, "ipv4 packet has invalid version (%d)",
In the original logic, when the Layer 3 protocol is IPv4, hlen equals sizeof(struct eth_header) + IP_HEADER_LEN. For IPv6, hlen equals sizeof(struct eth_header) + packet->l4_ofs - packet->l3_ofs. However, in the VLAN over VXLAN scenario, the hlen length does not include the VLAN length, leading to errors in popping the header. Now, hlen is uniformly modified to packet->l4_ofs, which includes the total length of both Layer 2 and Layer 3 headers, thereby correctly removing the tunnel header. Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com> --- lib/netdev-native-tnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)