| Message ID | 20200401183917.3620845-4-aconole@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | ip_frag: add a unit test for fragmentation | expand |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
> IPv6 only allows traffic source nodes to fragment, Yes. > so submitting > a packet with next header of IPPROTO_FRAGMENT would be invalid. If only source is allowed to fragment packet, then this check seems redundant, no? I can't imagine source calling fragment() twice for the same packet, and I don't see any point for us to check such situations. Besides, strictly speaking the check below is insufficient, as fragmentation ext header could be not the first one. Konstantin > > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > lib/librte_ip_frag/rte_ipv6_fragmentation.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c > index 820a5dc725..aebcfa4325 100644 > --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c > +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c > @@ -106,6 +106,10 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in, > > in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv6_hdr *); > > + /* Fragmenting a fragmented packet?! */ > + if (unlikely(in_hdr->proto == IPPROTO_FRAGMENT)) > + return -ENOTSUP; > + > in_seg = pkt_in; > in_seg_data_pos = sizeof(struct rte_ipv6_hdr); > out_pkt_pos = 0; > -- > 2.25.1
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> writes: >> IPv6 only allows traffic source nodes to fragment, > > Yes. > >> so submitting >> a packet with next header of IPPROTO_FRAGMENT would be invalid. > > If only source is allowed to fragment packet, then this check seems > redundant, no? Hrrm? How so? Is there something that prevents someone from calling the library function twice? > I can't imagine source calling fragment() twice for the same packet, and > I don't see any point for us to check such situations. Should we not check any error conditions at all? I don't understand. > Besides, strictly speaking the check below is insufficient, > as fragmentation ext header could be not the first one. You're right - we could probably walk next headers until we see one of the auth header types or upper layer header as "next header". I can respin with that if you'd like. > Konstantin > >> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> lib/librte_ip_frag/rte_ipv6_fragmentation.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c >> index 820a5dc725..aebcfa4325 100644 >> --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c >> +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c >> @@ -106,6 +106,10 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in, >> >> in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv6_hdr *); >> >> + /* Fragmenting a fragmented packet?! */ >> + if (unlikely(in_hdr->proto == IPPROTO_FRAGMENT)) >> + return -ENOTSUP; >> + >> in_seg = pkt_in; >> in_seg_data_pos = sizeof(struct rte_ipv6_hdr); >> out_pkt_pos = 0; >> -- >> 2.25.1
diff --git a/lib/librte_ip_frag/rte_ipv6_fragmentation.c b/lib/librte_ip_frag/rte_ipv6_fragmentation.c index 820a5dc725..aebcfa4325 100644 --- a/lib/librte_ip_frag/rte_ipv6_fragmentation.c +++ b/lib/librte_ip_frag/rte_ipv6_fragmentation.c @@ -106,6 +106,10 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in, in_hdr = rte_pktmbuf_mtod(pkt_in, struct rte_ipv6_hdr *); + /* Fragmenting a fragmented packet?! */ + if (unlikely(in_hdr->proto == IPPROTO_FRAGMENT)) + return -ENOTSUP; + in_seg = pkt_in; in_seg_data_pos = sizeof(struct rte_ipv6_hdr); out_pkt_pos = 0;
IPv6 only allows traffic source nodes to fragment, so submitting a packet with next header of IPPROTO_FRAGMENT would be invalid. Signed-off-by: Aaron Conole <aconole@redhat.com> --- lib/librte_ip_frag/rte_ipv6_fragmentation.c | 4 ++++ 1 file changed, 4 insertions(+)