| Message ID | 5411027934a79f0430edb905ad4b434ec6b8396e.1677888566.git.lucien.xin@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | netfilter: handle ipv6 jumbo packets properly for bridge ovs and tc | expand |
On Fri, Mar 03, 2023 at 07:12:41PM -0500, Xin Long wrote: > For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0, > and its real payload_len ( > 65535) is saved in hbh exthdr. With 0 > length for the jumbo packets, all data and exthdr will be trimmed > in nf_ct_skb_network_trim(). > > This patch is to call nf_ip6_check_hbh_len() to get real pkt_len > of the IPv6 packet, similar to br_validate_ipv6(). > > Signed-off-by: Xin Long <lucien.xin@gmail.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> ... > diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c > index 52b776bdf526..2016a3b05f86 100644 > --- a/net/netfilter/nf_conntrack_ovs.c > +++ b/net/netfilter/nf_conntrack_ovs.c ... > @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper); > int nf_ct_skb_network_trim(struct sk_buff *skb, int family) > { > unsigned int len; > + int err; > > switch (family) { > case NFPROTO_IPV4: > len = skb_ip_totlen(skb); > break; > case NFPROTO_IPV6: > - len = sizeof(struct ipv6hdr) > - + ntohs(ipv6_hdr(skb)->payload_len); > + len = ntohs(ipv6_hdr(skb)->payload_len); > + if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) { nit: if you spin a v2, you may consider reducing the scope of err to this block. > + err = nf_ip6_check_hbh_len(skb, &len); > + if (err) > + return err; > + } > + len += sizeof(struct ipv6hdr); > break; > default: > len = skb->len; > -- > 2.39.1 >
On 04/03/2023 02:12, Xin Long wrote: > For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0, > and its real payload_len ( > 65535) is saved in hbh exthdr. With 0 > length for the jumbo packets, all data and exthdr will be trimmed > in nf_ct_skb_network_trim(). > > This patch is to call nf_ip6_check_hbh_len() to get real pkt_len > of the IPv6 packet, similar to br_validate_ipv6(). > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/netfilter/nf_conntrack_ovs.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > I'd also recommend to change the scope of "err" as Simon already pointed out. Other than that looks good to me. Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Xin Long <lucien.xin@gmail.com> writes: > For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0, > and its real payload_len ( > 65535) is saved in hbh exthdr. With 0 > length for the jumbo packets, all data and exthdr will be trimmed > in nf_ct_skb_network_trim(). > > This patch is to call nf_ip6_check_hbh_len() to get real pkt_len > of the IPv6 packet, similar to br_validate_ipv6(). > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- Reviewed-by: Aaron Conole <aconole@redhat.com> > net/netfilter/nf_conntrack_ovs.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c > index 52b776bdf526..2016a3b05f86 100644 > --- a/net/netfilter/nf_conntrack_ovs.c > +++ b/net/netfilter/nf_conntrack_ovs.c > @@ -6,6 +6,7 @@ > #include <net/netfilter/ipv6/nf_defrag_ipv6.h> > #include <net/ipv6_frag.h> > #include <net/ip.h> > +#include <linux/netfilter_ipv6.h> > > /* 'skb' should already be pulled to nh_ofs. */ > int nf_ct_helper(struct sk_buff *skb, struct nf_conn *ct, > @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper); > int nf_ct_skb_network_trim(struct sk_buff *skb, int family) > { > unsigned int len; > + int err; > > switch (family) { > case NFPROTO_IPV4: > len = skb_ip_totlen(skb); > break; > case NFPROTO_IPV6: > - len = sizeof(struct ipv6hdr) > - + ntohs(ipv6_hdr(skb)->payload_len); > + len = ntohs(ipv6_hdr(skb)->payload_len); > + if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) { > + err = nf_ip6_check_hbh_len(skb, &len); > + if (err) > + return err; > + } > + len += sizeof(struct ipv6hdr); > break; > default: > len = skb->len;
On Mon, Mar 6, 2023 at 11:35 AM Simon Horman <simon.horman@corigine.com> wrote: > > On Fri, Mar 03, 2023 at 07:12:41PM -0500, Xin Long wrote: > > For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0, > > and its real payload_len ( > 65535) is saved in hbh exthdr. With 0 > > length for the jumbo packets, all data and exthdr will be trimmed > > in nf_ct_skb_network_trim(). > > > > This patch is to call nf_ip6_check_hbh_len() to get real pkt_len > > of the IPv6 packet, similar to br_validate_ipv6(). > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > Reviewed-by: Simon Horman <simon.horman@corigine.com> > > ... > > > diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c > > index 52b776bdf526..2016a3b05f86 100644 > > --- a/net/netfilter/nf_conntrack_ovs.c > > +++ b/net/netfilter/nf_conntrack_ovs.c > > ... > > > @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper); > > int nf_ct_skb_network_trim(struct sk_buff *skb, int family) > > { > > unsigned int len; > > + int err; > > > > switch (family) { > > case NFPROTO_IPV4: > > len = skb_ip_totlen(skb); > > break; > > case NFPROTO_IPV6: > > - len = sizeof(struct ipv6hdr) > > - + ntohs(ipv6_hdr(skb)->payload_len); > > + len = ntohs(ipv6_hdr(skb)->payload_len); > > + if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) { > > nit: if you spin a v2, > you may consider reducing the scope of err to this block. > Hi, Simon, I will post v2 with your suggestions including those in another 3 patches. :) Thanks. > > + err = nf_ip6_check_hbh_len(skb, &len); > > + if (err) > > + return err; > > + } > > + len += sizeof(struct ipv6hdr); > > break; > > default: > > len = skb->len; > > -- > > 2.39.1 > >
diff --git a/net/netfilter/nf_conntrack_ovs.c b/net/netfilter/nf_conntrack_ovs.c index 52b776bdf526..2016a3b05f86 100644 --- a/net/netfilter/nf_conntrack_ovs.c +++ b/net/netfilter/nf_conntrack_ovs.c @@ -6,6 +6,7 @@ #include <net/netfilter/ipv6/nf_defrag_ipv6.h> #include <net/ipv6_frag.h> #include <net/ip.h> +#include <linux/netfilter_ipv6.h> /* 'skb' should already be pulled to nh_ofs. */ int nf_ct_helper(struct sk_buff *skb, struct nf_conn *ct, @@ -114,14 +115,20 @@ EXPORT_SYMBOL_GPL(nf_ct_add_helper); int nf_ct_skb_network_trim(struct sk_buff *skb, int family) { unsigned int len; + int err; switch (family) { case NFPROTO_IPV4: len = skb_ip_totlen(skb); break; case NFPROTO_IPV6: - len = sizeof(struct ipv6hdr) - + ntohs(ipv6_hdr(skb)->payload_len); + len = ntohs(ipv6_hdr(skb)->payload_len); + if (ipv6_hdr(skb)->nexthdr == NEXTHDR_HOP) { + err = nf_ip6_check_hbh_len(skb, &len); + if (err) + return err; + } + len += sizeof(struct ipv6hdr); break; default: len = skb->len;
For IPv6 Jumbo packets, the ipv6_hdr(skb)->payload_len is always 0, and its real payload_len ( > 65535) is saved in hbh exthdr. With 0 length for the jumbo packets, all data and exthdr will be trimmed in nf_ct_skb_network_trim(). This patch is to call nf_ip6_check_hbh_len() to get real pkt_len of the IPv6 packet, similar to br_validate_ipv6(). Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/netfilter/nf_conntrack_ovs.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)