When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). Fixes: a7c528e5d71f ("net: add rte prefix to IP structure") Cc: olivier.matz@6wind.com Cc: stable@dpdk.org Signed-off-by: Pu Xu <583493798@qq.com> --- lib/librte_ip_frag/rte_ipv4_fragmentation.c | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c index e9de335ae..ee0049e09 100644 --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c @@ -23,10 +23,10 @@ #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, - uint16_t dofs, uint32_t mf) + const struct rte_ipv4_hdr *src, uint16_t header_len, + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) { - rte_memcpy(dst, src, sizeof(*dst)); + rte_memcpy(dst, src, header_len); fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); dst->fragment_offset = rte_cpu_to_be_16(fofs); @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, struct rte_ipv4_hdr *in_hdr; uint32_t out_pkt_pos, in_seg_data_pos; uint32_t more_in_segs; - uint16_t fragment_offset, flag_offset, frag_size; + uint16_t fragment_offset, flag_offset, frag_size, header_len; uint16_t frag_bytes_remaining; /* @@ -86,14 +86,21 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, unlikely(mtu_size < RTE_ETHER_MIN_MTU)) return -EINVAL; + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * RTE_IPV4_IHL_MULTIPLIER; + + /* Check IP header length */ + if (unlikely(pkt_in->data_len < header_len) || + unlikely(mtu_size < header_len)) + return -EINVAL; + /* * Ensure the IP payload length of all fragments is aligned to a * multiple of 8 bytes as per RFC791 section 2.3. */ - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), IPV4_HDR_FO_ALIGN); - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); /* If Don't Fragment flag is set */ @@ -102,11 +109,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, /* Check that pkts_out is big enough to hold all fragments */ if (unlikely(frag_size * nb_pkts_out < - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) + (uint16_t)(pkt_in->pkt_len - header_len))) return -EINVAL; in_seg = pkt_in; - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); + in_seg_data_pos = header_len; out_pkt_pos = 0; fragment_offset = 0; @@ -124,8 +131,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, } /* Reserve space for the IP header that will be built later */ - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); + out_pkt->data_len = header_len; + out_pkt->pkt_len = header_len; frag_bytes_remaining = frag_size; out_seg_prev = out_pkt; @@ -176,14 +183,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); - __fill_ipv4hdr_frag(out_hdr, in_hdr, + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, (uint16_t)out_pkt->pkt_len, flag_offset, fragment_offset, more_in_segs); fragment_offset = (uint16_t)(fragment_offset + - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); + out_pkt->pkt_len - header_len); - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); + out_pkt->l3_len = header_len; /* Write the fragment to the output list */ pkts_out[out_pkt_pos] = out_pkt; -- 2.17.0
> > When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct > rte_ipv4_hdr). Patch looks good to me... Though by some reason I couldn't find it at DPDK patchwork... http://patches.dpdk.org/project/dpdk/list/ Did you register at dev@dpdk.org? > > Fixes: a7c528e5d71f ("net: add rte prefix to IP structure") > Cc: olivier.matz@6wind.com > Cc: stable@dpdk.org > > Signed-off-by: Pu Xu <583493798@qq.com> > --- > lib/librte_ip_frag/rte_ipv4_fragmentation.c | 33 +++++++++++++-------- > 1 file changed, 20 insertions(+), 13 deletions(-) > > diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c > index e9de335ae..ee0049e09 100644 > --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c > +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c > @@ -23,10 +23,10 @@ > #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) > > static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, > - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, > - uint16_t dofs, uint32_t mf) > + const struct rte_ipv4_hdr *src, uint16_t header_len, > + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) > { > - rte_memcpy(dst, src, sizeof(*dst)); > + rte_memcpy(dst, src, header_len); > fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); > fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); > dst->fragment_offset = rte_cpu_to_be_16(fofs); > @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > struct rte_ipv4_hdr *in_hdr; > uint32_t out_pkt_pos, in_seg_data_pos; > uint32_t more_in_segs; > - uint16_t fragment_offset, flag_offset, frag_size; > + uint16_t fragment_offset, flag_offset, frag_size, header_len; > uint16_t frag_bytes_remaining; > > /* > @@ -86,14 +86,21 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > unlikely(mtu_size < RTE_ETHER_MIN_MTU)) > return -EINVAL; > > + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); > + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * RTE_IPV4_IHL_MULTIPLIER; > + > + /* Check IP header length */ > + if (unlikely(pkt_in->data_len < header_len) || > + unlikely(mtu_size < header_len)) > + return -EINVAL; > + > /* > * Ensure the IP payload length of all fragments is aligned to a > * multiple of 8 bytes as per RFC791 section 2.3. > */ > - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), > + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), > IPV4_HDR_FO_ALIGN); > > - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); > flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); > > /* If Don't Fragment flag is set */ > @@ -102,11 +109,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > > /* Check that pkts_out is big enough to hold all fragments */ > if (unlikely(frag_size * nb_pkts_out < > - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) > + (uint16_t)(pkt_in->pkt_len - header_len))) > return -EINVAL; > > in_seg = pkt_in; > - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); > + in_seg_data_pos = header_len; > out_pkt_pos = 0; > fragment_offset = 0; > > @@ -124,8 +131,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > } > > /* Reserve space for the IP header that will be built later */ > - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); > - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); > + out_pkt->data_len = header_len; > + out_pkt->pkt_len = header_len; > frag_bytes_remaining = frag_size; > > out_seg_prev = out_pkt; > @@ -176,14 +183,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > > out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); > > - __fill_ipv4hdr_frag(out_hdr, in_hdr, > + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, > (uint16_t)out_pkt->pkt_len, > flag_offset, fragment_offset, more_in_segs); > > fragment_offset = (uint16_t)(fragment_offset + > - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); > + out_pkt->pkt_len - header_len); > > - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); > + out_pkt->l3_len = header_len; > > /* Write the fragment to the output list */ > pkts_out[out_pkt_pos] = out_pkt; > -- > 2.17.0 >
When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). Fixes: a7c528e5d71f ("net: add rte prefix to IP structure") Cc: olivier.matz@6wind.com Cc: stable@dpdk.org Signed-off-by: Pu Xu <583493798@qq.com> --- lib/librte_ip_frag/rte_ipv4_fragmentation.c | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c index e9de335ae..ee0049e09 100644 --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c @@ -23,10 +23,10 @@ #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, - uint16_t dofs, uint32_t mf) + const struct rte_ipv4_hdr *src, uint16_t header_len, + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) { - rte_memcpy(dst, src, sizeof(*dst)); + rte_memcpy(dst, src, header_len); fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); dst->fragment_offset = rte_cpu_to_be_16(fofs); @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, struct rte_ipv4_hdr *in_hdr; uint32_t out_pkt_pos, in_seg_data_pos; uint32_t more_in_segs; - uint16_t fragment_offset, flag_offset, frag_size; + uint16_t fragment_offset, flag_offset, frag_size, header_len; uint16_t frag_bytes_remaining; /* @@ -86,14 +86,21 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, unlikely(mtu_size < RTE_ETHER_MIN_MTU)) return -EINVAL; + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * RTE_IPV4_IHL_MULTIPLIER; + + /* Check IP header length */ + if (unlikely(pkt_in->data_len < header_len) || + unlikely(mtu_size < header_len)) + return -EINVAL; + /* * Ensure the IP payload length of all fragments is aligned to a * multiple of 8 bytes as per RFC791 section 2.3. */ - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), IPV4_HDR_FO_ALIGN); - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); /* If Don't Fragment flag is set */ @@ -102,11 +109,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, /* Check that pkts_out is big enough to hold all fragments */ if (unlikely(frag_size * nb_pkts_out < - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) + (uint16_t)(pkt_in->pkt_len - header_len))) return -EINVAL; in_seg = pkt_in; - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); + in_seg_data_pos = header_len; out_pkt_pos = 0; fragment_offset = 0; @@ -124,8 +131,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, } /* Reserve space for the IP header that will be built later */ - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); + out_pkt->data_len = header_len; + out_pkt->pkt_len = header_len; frag_bytes_remaining = frag_size; out_seg_prev = out_pkt; @@ -176,14 +183,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); - __fill_ipv4hdr_frag(out_hdr, in_hdr, + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, (uint16_t)out_pkt->pkt_len, flag_offset, fragment_offset, more_in_segs); fragment_offset = (uint16_t)(fragment_offset + - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); + out_pkt->pkt_len - header_len); - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); + out_pkt->l3_len = header_len; /* Write the fragment to the output list */ pkts_out[out_pkt_pos] = out_pkt; -- 2.17.0
When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). Fixes: a7c528e5d71f ("net: add rte prefix to IP structure") Cc: olivier.matz@6wind.com Cc: stable@dpdk.org Signed-off-by: Pu Xu <583493798@qq.com> --- lib/librte_ip_frag/rte_ipv4_fragmentation.c | 33 +++++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c index e9de335ae..ee0049e09 100644 --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c @@ -23,10 +23,10 @@ #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, - uint16_t dofs, uint32_t mf) + const struct rte_ipv4_hdr *src, uint16_t header_len, + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) { - rte_memcpy(dst, src, sizeof(*dst)); + rte_memcpy(dst, src, header_len); fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); dst->fragment_offset = rte_cpu_to_be_16(fofs); @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, struct rte_ipv4_hdr *in_hdr; uint32_t out_pkt_pos, in_seg_data_pos; uint32_t more_in_segs; - uint16_t fragment_offset, flag_offset, frag_size; + uint16_t fragment_offset, flag_offset, frag_size, header_len; uint16_t frag_bytes_remaining; /* @@ -86,14 +86,21 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, unlikely(mtu_size < RTE_ETHER_MIN_MTU)) return -EINVAL; + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * RTE_IPV4_IHL_MULTIPLIER; + + /* Check IP header length */ + if (unlikely(pkt_in->data_len < header_len) || + unlikely(mtu_size < header_len)) + return -EINVAL; + /* * Ensure the IP payload length of all fragments is aligned to a * multiple of 8 bytes as per RFC791 section 2.3. */ - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), IPV4_HDR_FO_ALIGN); - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); /* If Don't Fragment flag is set */ @@ -102,11 +109,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, /* Check that pkts_out is big enough to hold all fragments */ if (unlikely(frag_size * nb_pkts_out < - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) + (uint16_t)(pkt_in->pkt_len - header_len))) return -EINVAL; in_seg = pkt_in; - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); + in_seg_data_pos = header_len; out_pkt_pos = 0; fragment_offset = 0; @@ -124,8 +131,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, } /* Reserve space for the IP header that will be built later */ - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); + out_pkt->data_len = header_len; + out_pkt->pkt_len = header_len; frag_bytes_remaining = frag_size; out_seg_prev = out_pkt; @@ -176,14 +183,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); - __fill_ipv4hdr_frag(out_hdr, in_hdr, + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, (uint16_t)out_pkt->pkt_len, flag_offset, fragment_offset, more_in_segs); fragment_offset = (uint16_t)(fragment_offset + - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); + out_pkt->pkt_len - header_len); - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); + out_pkt->l3_len = header_len; /* Write the fragment to the output list */ pkts_out[out_pkt_pos] = out_pkt; -- 2.17.0
Hi, > When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct > rte_ipv4_hdr). There are few checkpatch errro/warnings: http://mails.dpdk.org/archives/test-report/2020-May/129803.html please fix. > > Fixes: a7c528e5d71f ("net: add rte prefix to IP structure") AFAIK, this commit didn't change anything in ip_frag code. It was just a mechanic renaming for some public structs. Commit-id that you provided in your previous patch version Seems more relevant. With these things fixed: Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > Cc: olivier.matz@6wind.com > Cc: stable@dpdk.org > > Signed-off-by: Pu Xu <583493798@qq.com> > --- > lib/librte_ip_frag/rte_ipv4_fragmentation.c | 33 +++++++++++++-------- > 1 file changed, 20 insertions(+), 13 deletions(-) > > diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c > index e9de335ae..ee0049e09 100644 > --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c > +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c > @@ -23,10 +23,10 @@ > #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) > > static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, > - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, > - uint16_t dofs, uint32_t mf) > + const struct rte_ipv4_hdr *src, uint16_t header_len, > + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) > { > - rte_memcpy(dst, src, sizeof(*dst)); > + rte_memcpy(dst, src, header_len); > fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); > fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); > dst->fragment_offset = rte_cpu_to_be_16(fofs); > @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > struct rte_ipv4_hdr *in_hdr; > uint32_t out_pkt_pos, in_seg_data_pos; > uint32_t more_in_segs; > - uint16_t fragment_offset, flag_offset, frag_size; > + uint16_t fragment_offset, flag_offset, frag_size, header_len; > uint16_t frag_bytes_remaining; > > /* > @@ -86,14 +86,21 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > unlikely(mtu_size < RTE_ETHER_MIN_MTU)) > return -EINVAL; > > + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); > + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * RTE_IPV4_IHL_MULTIPLIER; > + > + /* Check IP header length */ > + if (unlikely(pkt_in->data_len < header_len) || > + unlikely(mtu_size < header_len)) > + return -EINVAL; > + > /* > * Ensure the IP payload length of all fragments is aligned to a > * multiple of 8 bytes as per RFC791 section 2.3. > */ > - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), > + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), > IPV4_HDR_FO_ALIGN); > > - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); > flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); > > /* If Don't Fragment flag is set */ > @@ -102,11 +109,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > > /* Check that pkts_out is big enough to hold all fragments */ > if (unlikely(frag_size * nb_pkts_out < > - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) > + (uint16_t)(pkt_in->pkt_len - header_len))) > return -EINVAL; > > in_seg = pkt_in; > - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); > + in_seg_data_pos = header_len; > out_pkt_pos = 0; > fragment_offset = 0; > > @@ -124,8 +131,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > } > > /* Reserve space for the IP header that will be built later */ > - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); > - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); > + out_pkt->data_len = header_len; > + out_pkt->pkt_len = header_len; > frag_bytes_remaining = frag_size; > > out_seg_prev = out_pkt; > @@ -176,14 +183,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, > > out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); > > - __fill_ipv4hdr_frag(out_hdr, in_hdr, > + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, > (uint16_t)out_pkt->pkt_len, > flag_offset, fragment_offset, more_in_segs); > > fragment_offset = (uint16_t)(fragment_offset + > - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); > + out_pkt->pkt_len - header_len); > > - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); > + out_pkt->l3_len = header_len; > > /* Write the fragment to the output list */ > pkts_out[out_pkt_pos] = out_pkt; > -- > 2.17.0 >
When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). Fixes: 4c38e5532a07 ("ip_frag: refactor IPv4 fragmentation into a proper library") Cc: anatoly.burakov@intel.com Cc: stable@dpdk.org --- lib/librte_ip_frag/rte_ipv4_fragmentation.c | 34 +++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c index e9de335ae..2e7739d02 100644 --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c @@ -23,10 +23,10 @@ #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, - uint16_t dofs, uint32_t mf) + const struct rte_ipv4_hdr *src, uint16_t header_len, + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) { - rte_memcpy(dst, src, sizeof(*dst)); + rte_memcpy(dst, src, header_len); fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); dst->fragment_offset = rte_cpu_to_be_16(fofs); @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, struct rte_ipv4_hdr *in_hdr; uint32_t out_pkt_pos, in_seg_data_pos; uint32_t more_in_segs; - uint16_t fragment_offset, flag_offset, frag_size; + uint16_t fragment_offset, flag_offset, frag_size, header_len; uint16_t frag_bytes_remaining; /* @@ -86,14 +86,22 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, unlikely(mtu_size < RTE_ETHER_MIN_MTU)) return -EINVAL; + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * + RTE_IPV4_IHL_MULTIPLIER; + + /* Check IP header length */ + if (unlikely(pkt_in->data_len < header_len) || + unlikely(mtu_size < header_len)) + return -EINVAL; + /* * Ensure the IP payload length of all fragments is aligned to a * multiple of 8 bytes as per RFC791 section 2.3. */ - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), IPV4_HDR_FO_ALIGN); - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); /* If Don't Fragment flag is set */ @@ -102,11 +110,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, /* Check that pkts_out is big enough to hold all fragments */ if (unlikely(frag_size * nb_pkts_out < - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) + (uint16_t)(pkt_in->pkt_len - header_len))) return -EINVAL; in_seg = pkt_in; - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); + in_seg_data_pos = header_len; out_pkt_pos = 0; fragment_offset = 0; @@ -124,8 +132,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, } /* Reserve space for the IP header that will be built later */ - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); + out_pkt->data_len = header_len; + out_pkt->pkt_len = header_len; frag_bytes_remaining = frag_size; out_seg_prev = out_pkt; @@ -176,14 +184,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); - __fill_ipv4hdr_frag(out_hdr, in_hdr, + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, (uint16_t)out_pkt->pkt_len, flag_offset, fragment_offset, more_in_segs); fragment_offset = (uint16_t)(fragment_offset + - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); + out_pkt->pkt_len - header_len); - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); + out_pkt->l3_len = header_len; /* Write the fragment to the output list */ pkts_out[out_pkt_pos] = out_pkt; -- 2.17.0
When fragmenting ipv4 packet, the data offset should be calculated through the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). Fixes: 4c38e5532a07 ("ip_frag: refactor IPv4 fragmentation into a proper library") Cc: anatoly.burakov@intel.com Cc: stable@dpdk.org Signed-off-by: Pu Xu <583493798@qq.com> --- lib/librte_ip_frag/rte_ipv4_fragmentation.c | 34 +++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/librte_ip_frag/rte_ipv4_fragmentation.c b/lib/librte_ip_frag/rte_ipv4_fragmentation.c index e9de335ae..2e7739d02 100644 --- a/lib/librte_ip_frag/rte_ipv4_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv4_fragmentation.c @@ -23,10 +23,10 @@ #define IPV4_HDR_FO_ALIGN (1 << RTE_IPV4_HDR_FO_SHIFT) static inline void __fill_ipv4hdr_frag(struct rte_ipv4_hdr *dst, - const struct rte_ipv4_hdr *src, uint16_t len, uint16_t fofs, - uint16_t dofs, uint32_t mf) + const struct rte_ipv4_hdr *src, uint16_t header_len, + uint16_t len, uint16_t fofs, uint16_t dofs, uint32_t mf) { - rte_memcpy(dst, src, sizeof(*dst)); + rte_memcpy(dst, src, header_len); fofs = (uint16_t)(fofs + (dofs >> RTE_IPV4_HDR_FO_SHIFT)); fofs = (uint16_t)(fofs | mf << RTE_IPV4_HDR_MF_SHIFT); dst->fragment_offset = rte_cpu_to_be_16(fofs); @@ -74,7 +74,7 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, struct rte_ipv4_hdr *in_hdr; uint32_t out_pkt_pos, in_seg_data_pos; uint32_t more_in_segs; - uint16_t fragment_offset, flag_offset, frag_size; + uint16_t fragment_offset, flag_offset, frag_size, header_len; uint16_t frag_bytes_remaining; /* @@ -86,14 +86,22 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, unlikely(mtu_size < RTE_ETHER_MIN_MTU)) return -EINVAL; + in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); + header_len = (in_hdr->version_ihl & RTE_IPV4_HDR_IHL_MASK) * + RTE_IPV4_IHL_MULTIPLIER; + + /* Check IP header length */ + if (unlikely(pkt_in->data_len < header_len) || + unlikely(mtu_size < header_len)) + return -EINVAL; + /* * Ensure the IP payload length of all fragments is aligned to a * multiple of 8 bytes as per RFC791 section 2.3. */ - frag_size = RTE_ALIGN_FLOOR((mtu_size - sizeof(struct rte_ipv4_hdr)), + frag_size = RTE_ALIGN_FLOOR((mtu_size - header_len), IPV4_HDR_FO_ALIGN); - in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv4_hdr *); flag_offset = rte_cpu_to_be_16(in_hdr->fragment_offset); /* If Don't Fragment flag is set */ @@ -102,11 +110,11 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, /* Check that pkts_out is big enough to hold all fragments */ if (unlikely(frag_size * nb_pkts_out < - (uint16_t)(pkt_in->pkt_len - sizeof(struct rte_ipv4_hdr)))) + (uint16_t)(pkt_in->pkt_len - header_len))) return -EINVAL; in_seg = pkt_in; - in_seg_data_pos = sizeof(struct rte_ipv4_hdr); + in_seg_data_pos = header_len; out_pkt_pos = 0; fragment_offset = 0; @@ -124,8 +132,8 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, } /* Reserve space for the IP header that will be built later */ - out_pkt->data_len = sizeof(struct rte_ipv4_hdr); - out_pkt->pkt_len = sizeof(struct rte_ipv4_hdr); + out_pkt->data_len = header_len; + out_pkt->pkt_len = header_len; frag_bytes_remaining = frag_size; out_seg_prev = out_pkt; @@ -176,14 +184,14 @@ rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in, out_hdr = rte_pktmbuf_mtod(out_pkt, struct rte_ipv4_hdr *); - __fill_ipv4hdr_frag(out_hdr, in_hdr, + __fill_ipv4hdr_frag(out_hdr, in_hdr, header_len, (uint16_t)out_pkt->pkt_len, flag_offset, fragment_offset, more_in_segs); fragment_offset = (uint16_t)(fragment_offset + - out_pkt->pkt_len - sizeof(struct rte_ipv4_hdr)); + out_pkt->pkt_len - header_len); - out_pkt->l3_len = sizeof(struct rte_ipv4_hdr); + out_pkt->l3_len = header_len; /* Write the fragment to the output list */ pkts_out[out_pkt_pos] = out_pkt; -- 2.17.0
> > When fragmenting ipv4 packet, the data offset should be calculated through > the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). > > Fixes: 4c38e5532a07 ("ip_frag: refactor IPv4 fragmentation into a proper library") > Cc: anatoly.burakov@intel.com > Cc: stable@dpdk.org > --- Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > 2.17.0 >
01/05/2020 22:43, Ananyev, Konstantin: > > > > > When fragmenting ipv4 packet, the data offset should be calculated through > > the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr). > > > > Fixes: 4c38e5532a07 ("ip_frag: refactor IPv4 fragmentation into a proper library") > > Cc: anatoly.burakov@intel.com > > Cc: stable@dpdk.org > > --- > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> The patch cannot be merged because it is missing a Signed-off-by line. In general, please follow the contributions guideline here: http://core.dpdk.org/contribute/#send Would be appreciated to number subsequent versions of the patches, and use --in-reply-to to keep them threaded.
24/05/2020 17:27, Thomas Monjalon:
> 01/05/2020 22:43, Ananyev, Konstantin:
> >
> > >
> > > When fragmenting ipv4 packet, the data offset should be calculated through
> > > the ihl field in ip header rather than using sizeof(struct rte_ipv4_hdr).
> > >
> > > Fixes: 4c38e5532a07 ("ip_frag: refactor IPv4 fragmentation into a proper library")
> > > Cc: anatoly.burakov@intel.com
> > > Cc: stable@dpdk.org
> > > ---
> >
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
> The patch cannot be merged because it is missing a Signed-off-by line.
>
> In general, please follow the contributions guideline here:
> http://core.dpdk.org/contribute/#send
>
> Would be appreciated to number subsequent versions of the patches,
> and use --in-reply-to to keep them threaded.
Are we going to celebrate one year birthday of this patch? :)
How can we proceed?