From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Burakov, Anatoly" Subject: Re: [PATCH v2 2/2] ip_frag: use key length for key comparision Date: Tue, 6 Nov 2018 10:53:45 +0000 Message-ID: References: <1541413603-4792-1-git-send-email-konstantin.ananyev@intel.com> <1541420338-300-3-git-send-email-konstantin.ananyev@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: stable@dpdk.org, ryan.e.hall@intel.com, alexander.v.gutkin@intel.com To: Konstantin Ananyev , dev@dpdk.org Return-path: In-Reply-To: <1541420338-300-3-git-send-email-konstantin.ananyev@intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 05-Nov-18 12:18 PM, Konstantin Ananyev wrote: > Right now reassembly code relies on src_dst[] being all zeroes to > determine is it free/occupied entry in the fragments table. > This is suboptimal and error prone - user can crash DPDK ip_reassembly > app by something like the following scapy script: > x=Ether(src=...,dst=...)/IP(dst='0.0.0.0',src='0.0.0.0',id=0)/('X'*1000) > frags=fragment(x, fragsize=500) > sendp(frags, iface=...) > To overcome that issue and reduce overhead of > 'key invalidate' and 'key is empty' operations - > add key_len into keys comparision procedure. > > Fixes: 4f1a8f633862 ("ip_frag: add IPv6 reassembly") > Cc: stable@dpdk.org > > Reported-by: Ryan E Hall > Reported-by: Alexander V Gutkin > Signed-off-by: Konstantin Ananyev > --- > @@ -44,9 +44,17 @@ struct ip_frag { > > /** @internal to uniquely identify fragmented datagram. */ > struct ip_frag_key { > - uint64_t src_dst[4]; /**< src address, first 8 bytes used for IPv4 */ > - uint32_t id; /**< dst address */ > - uint32_t key_len; /**< src/dst key length */ > + uint64_t src_dst[4]; > + /**< src and dst address, only first 8 bytes used for IPv4 */ > + RTE_STD_C11 > + union { > + uint64_t id_key_len; /**< combined for easy fetch */ > + __extension__ > + struct { > + uint32_t id; /**< packet id */ > + uint32_t key_len; /**< src/dst key length */ > + }; > + }; > }; Would that break ABI? > > /** > -- Thanks, Anatoly