From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 45628A3160 for ; Wed, 9 Oct 2019 19:24:43 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8BA351C220; Wed, 9 Oct 2019 19:24:42 +0200 (CEST) Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com [209.85.215.196]) by dpdk.org (Postfix) with ESMTP id 70F1F1C21E for ; Wed, 9 Oct 2019 19:24:40 +0200 (CEST) Received: by mail-pg1-f196.google.com with SMTP id y35so1849801pgl.1 for ; Wed, 09 Oct 2019 10:24:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=b/rOZj1kTB6bigAVxFl1C8b3phjMxLg3YSYCnihDXTY=; b=DyHvNdobtSSxzDkq9+keG0bhSWrcwTu7nsFJ7QZoxowN8H6R7GDiy6iCQ/tDyakHD/ IXZmUYsCrVCJhZFsN13fm1z4tPRezZ+6GnIBDrCkBbrjKpGIHMzIvaHgstDuJviykPbO fnfzHJw6aHcogJFxsoibK8oMm0sQqaCaTjWyR+3LUVRrbHKvykByXe7QzEal/5q6rNhj 9mlEFnnmzxAxFh/1ROYIwKmxCaDQXfHtLNfqMIBbYEzB9EkW9hsDPv6eNeVnTFLETdQV UsU4CtQ+cAF9G9dA6b35PuPPr6uNelMi+DdPDCduTI2r11rN/flhMbl0DxG6hlM5Z/Fv dkPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=b/rOZj1kTB6bigAVxFl1C8b3phjMxLg3YSYCnihDXTY=; b=M7wgUVHpxou1Iig4aV4bbJo4/ay69iwG9P9SvldPiolAsprSu0z9k0TNOZZ15VpXeK CguwybqotTL0F0QpkqFsEQ4cT9YmiWlTz7KUOKUyRbZjKcL3P1Og5LTZfqhbP82NzMFs HLL1jayNMRyJje0hez7xsoHRAi0vbyq3rV4pE2g0PDcbE98cy02OTvBUydoL9UQ0Pdp5 pIsOQ1Bbk6WJRQTvVzkjvESBB8QRoYcY7OlpxobBq/VktRcE6jIJ0zOfBLQW1x3pdpvQ 7jy4j5Nh97KOTAMCZkTJX2EGsRoFBOZwdGARsB2ZBMS+7nghBM2bUoBOWb4vrI0dKIx1 AriA== X-Gm-Message-State: APjAAAWgFixB8+SmKrEg0P7ta07imw/GSwc1sPuskdB85rzYDPf5sflf /jSFwMHcyRBMVm0FKlc1D7Vkig== X-Google-Smtp-Source: APXvYqxvY/Y4pcAGWfMq39OWGvD3J7SypAvqagYY9l6PHdRR8/TRftdtG3Wxtl6rdNiYePhaMNDFYQ== X-Received: by 2002:a17:90b:d90:: with SMTP id bg16mr5552325pjb.143.1570641879407; Wed, 09 Oct 2019 10:24:39 -0700 (PDT) Received: from hermes.lan (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id t3sm7110086pje.7.2019.10.09.10.24.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Oct 2019 10:24:39 -0700 (PDT) Date: Wed, 9 Oct 2019 10:24:32 -0700 From: Stephen Hemminger To: Morten =?UTF-8?B?QnLDuHJ1cA==?= Cc: "Ananyev, Konstantin" , "dpdk-dev" , "Jerin Jacob" Message-ID: <20191009102432.1199e792@hermes.lan> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C60B6A@smartserver.smartshare.dk> References: <98CBD80474FA8B44BF855DF32C47DC35C60B67@smartserver.smartshare.dk> <2601191342CEEE43887BDE71AB977258019197431F@irsmsx105.ger.corp.intel.com> <20191009080218.3711bef3@hermes.lan> <98CBD80474FA8B44BF855DF32C47DC35C60B69@smartserver.smartshare.dk> <20191009081442.3e7f1af4@hermes.lan> <98CBD80474FA8B44BF855DF32C47DC35C60B6A@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] packet data access bug in bpf and pdump libs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list 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 Wed, 9 Oct 2019 17:20:58 +0200 Morten Br=C3=B8rup wrote: > > -----Original Message----- > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > Sent: Wednesday, October 9, 2019 5:15 PM > >=20 > > On Wed, 9 Oct 2019 17:06:24 +0200 > > Morten Br=C3=B8rup wrote: > > =20 > > > > -----Original Message----- > > > > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > > > > Sent: Wednesday, October 9, 2019 5:02 PM > > > > > > > > On Wed, 9 Oct 2019 11:11:46 +0000 > > > > "Ananyev, Konstantin" wrote: > > > > =20 > > > > > Hi Morten, > > > > > =20 > > > > > > > > > > > > Hi Konstantin and Stephen, > > > > > > > > > > > > I just noticed the same bug in your bpf and pcap libraries: > > > > > > > > > > > > You are using rte_pktmbuf_mtod(), but should be using =20 > > > > rte_pktmbuf_read(). Otherwise you cannot read data across multiple > > > > segments. =20 > > > > > > > > > > In plain data buffer mode expected input for BPF program is start= =20 > > of =20 > > > > first segment packet data. =20 > > > > > Other segments are simply not available to BPF program in that =20 > > mode. =20 > > > > > AFAIK, cBPF uses the same model. > > > > > =20 > > > > > > > > > > > > > > > > > > Med venlig hilsen / kind regards > > > > > > - Morten Br=C3=B8rup =20 > > > > > =20 > > > > > > > > For packet capture, the BPF program is only allowed to look at =20 > > first =20 > > > > segment. > > > > pktmbuf_read is expensive and can cause a copy. =20 > > > > > > It is only expensive if going beyond the first segment: > > > > > > static inline const void *rte_pktmbuf_read(const struct rte_mbuf *m, > > > uint32_t off, uint32_t len, void *buf) > > > { > > > if (likely(off + len <=3D rte_pktmbuf_data_len(m))) > > > return rte_pktmbuf_mtod_offset(m, char *, off); > > > else > > > return __rte_pktmbuf_read(m, off, len, buf); > > > } =20 > >=20 > > But it would mean potentially big buffer on the stack (in case) =20 >=20 > No, the buffer only needs to be the size of the accessed data. I use it l= ike this: >=20 > char buffer[sizeof(uint32_t)]; >=20 > for (;; pc++) { > switch (pc->code) { > case BPF_LD_ABS_32: > p =3D rte_pktmbuf_read(m, pc->k, sizeof(uint32_t), buffer); > if (unlikely(p =3D=3D NULL)) return 0; /* Attempting to read = beyond packet. Bail out. */ > a =3D rte_be_to_cpu_32(*(const uint32_t *)p); > continue; > case BPF_LD_ABS_16: > p =3D rte_pktmbuf_read(m, pc->k, sizeof(uint16_t), buffer); > if (unlikely(p =3D=3D NULL)) return 0; /* Attempting to read = beyond packet. Bail out. */ > a =3D rte_be_to_cpu_16(*(const uint16_t *)p); > continue; >=20 Reading down the chain of mbuf segments to find a uint32_t (and that potent= ially crosses) seems like a waste. The purpose of the filter is to look at packet headers. Any driver making m= bufs that are dripples of data is broken. chaining is really meant for case of jumbo = or tso.