[v3,3/4] ip_frag: ipv6 fragments must not be resubmitted to fragmentation
diff mbox series

Message ID 20200401183917.3620845-4-aconole@redhat.com
State Superseded, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • ip_frag: add a unit test for fragmentation
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/checkpatch success coding style OK

Commit Message

Aaron Conole April 1, 2020, 6:39 p.m. UTC
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(+)

Comments

Ananyev, Konstantin April 7, 2020, 10:43 a.m. UTC | #1
> 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
Aaron Conole April 7, 2020, 12:40 p.m. UTC | #2
"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

Patch
diff mbox series

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;