DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Huichao Cai <chcchc88@163.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: Re:Re: [PATCH] ip_frag: add IPv4 options fragment and unit test data
Date: Fri, 11 Feb 2022 09:41:27 +0000	[thread overview]
Message-ID: <DM6PR11MB4491192DBFDB3B9E595138BA9A309@DM6PR11MB4491.namprd11.prod.outlook.com> (raw)
In-Reply-To: <2b84245.b0e.17ee68dba31.Coremail.chcchc88@163.com>


Hi Huichao

>>As a nit, why not 'uint8_t *', to keep style the same through all file?
>Yes,I can use 'uint8_t *.Thank you for your correction.
>
>>We already done such calculation in rte_ipv4_fragment_packet(),
>>so can re-use header_len value here.
>Yes,I can re-use header_len.Thank you for your correction.
>
>>Why optlen==1 is not considered as valid one?
>RFC791:
>        Case 2:  An option-type octet, an option-length octet, and the actual option-data octets.
>       The option-length octet counts the option-type octet and the option-length octet as well as the option-data octets.
>So for case 2, the value of optlen is at least 2.

Ok, thanks for explanation.

>>What means 'ANK' here? 
>Because this code comes from the Linux kernel and is licensed under the GPL, I kept the original comments, I looked up the Linux kernel code, and ‘ANK’ should be the name of a >developer.

AFAIK, we can't copy-paste code from Linux kernel.
As you noted it is under GPL, while DPDK is under BSD-3 license. 

>>I see two problems with that approach:
>>- you modify incoming packet's header - which is the change in behaviour,
>>  and doesn't look right at all.
>Because the incoming packet is fragmented, the incoming packet IP header is no longer used, so this behavior does not cause problems.At the same time, in order to consider >efficiency, modify the original IP header directly, rather than creating a separate IP header.Of course, if this method is not reasonable, I can create a separate IP header to modify.

Library routine has no idea would original IP packet will be used later or not.
In your particular case it might be not needed, but there might be other usages,
that do use it (logging, send un-fragmented via other port, etc.).
So I think we have to preserve original behaviour.

>>- you remove not-copied options from all fragments.
>>  As I can read RFC 791 - first fragment should have a copy of all options present
>>  in original packet, while other fragments need to have only those that have to be
>>  copied.  
>This function(ip_options_fragment) is called under that ‘__fill_ipv4hdr_frag’,so the first fragment have a copy of all options present in original packet.

Right, I missed the fact that you do modify original packet after making a first copy.
 

  reply	other threads:[~2022-02-11  9:41 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-10 12:21 Ananyev, Konstantin
2022-02-11  2:12 ` Huichao Cai
2022-02-11  9:41   ` Ananyev, Konstantin [this message]
2022-02-11 10:00     ` Huichao Cai
2022-02-11  2:20 ` Huichao Cai
2022-02-11 10:11   ` Ferruh Yigit
  -- strict thread matches above, loose matches on Subject: below --
2021-12-02  9:35 Dariusz Sosnowski
2021-12-02 11:38 ` Huichao Cai
2021-12-02 12:03   ` Ananyev, Konstantin
2021-11-24  8:47 Huichao Cai
2021-12-01 11:49 ` Dariusz Sosnowski
2021-12-02  2:24   ` Huichao Cai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB4491192DBFDB3B9E595138BA9A309@DM6PR11MB4491.namprd11.prod.outlook.com \
    --to=konstantin.ananyev@intel.com \
    --cc=chcchc88@163.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).