From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f173.google.com (mail-pf0-f173.google.com [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id CB8A99E7 for ; Thu, 22 Sep 2016 03:44:55 +0200 (CEST) Received: by mail-pf0-f173.google.com with SMTP id p64so24864958pfb.1 for ; Wed, 21 Sep 2016 18:44:55 -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=eCtw/nyu63GDW8mj00JWPy5hE5Q+qf6qcQMzPBk9yhE=; b=irWwnDRqcKMt1uv4OoUpF4yq9CtEu8qoJ1erS2gCoqSg0JhxrpXKc360gk8QRVykTb Co2Fv3Rq/17skXKJTc+X5LuGnEG7LdAnR8TI3AMAKSITsjS88jPMUpbqMfVTOyST1vsk V/7/cN80OdryrvWXTIY7Nx7wKgEnsu6/ExAUlYojkkhV+TRpuCGPBEJ3/IKqZSuZ2t3J vr82h86SzpZgHf/qTV9P7q6RaqWAPQcTlyZH0OPxyM0h/ypIc+/n+rvkx30uvVCNU2fj qb1hLCk6ZUwQX6Xy9s6dIT6CU/A4DFZUAOyoUhnkLsb830O20Qq3GyeLkvQY+VIFqlIh 7xBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=eCtw/nyu63GDW8mj00JWPy5hE5Q+qf6qcQMzPBk9yhE=; b=QuhjBCx1D79RT+DZCfDbYp23KQDqeRRXUBsQMKb38g78ID7DVxy2GXk6ftvLIlpGVp aldyPKBYeRltaH4SUu66fKg+mRccBctHW9toixdKnFfgQQPZCwOYpqEo2jrNQRCuv1St ZA8LGwjzhGviuoLBTcVdyEod315cq9oNoINY0y2LSSV1xF0MWVP3DkN6kOu/NFeCDBSW TUpp41kNB5luqgTBsCkDOQl4uUVJa3MJV0duz0N2Y5hTNp1Qef1aMFFC8h8PflLAbf+I zKUaOaJjmtkUb0KJvnpy3IMvTs5xcVaseAXI82CW51KfjIponw4AspxokPOpa5OYVrOH KqmA== X-Gm-Message-State: AE9vXwP/9FFPbV+TPYSbJplW1wkYdFHDY45uWC8uzFXHaM00/zm6g88PDUUu8gbER5g0Bw== X-Received: by 10.98.28.146 with SMTP id c140mr66067316pfc.158.1474508695182; Wed, 21 Sep 2016 18:44:55 -0700 (PDT) Received: from xeon-e3 (static-50-53-69-251.bvtn.or.frontiernet.net. [50.53.69.251]) by smtp.gmail.com with ESMTPSA id jh3sm4737675pac.16.2016.09.21.18.44.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 21 Sep 2016 18:44:55 -0700 (PDT) Date: Wed, 21 Sep 2016 18:45:05 -0700 From: Stephen Hemminger To: "Dey, Souvik" Cc: "mark.b.kavanagh@intel.com" , "yuanhan.liu@linux.intel.com" , "dev@dpdk.org" Message-ID: <20160921184505.584367ef@xeon-e3> In-Reply-To: References: <20160921231147.26820-1-sodey@sonusnet.com> <20160921162213.4b79d1ce@xeon-e3> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6] net/virtio: add set_mtu in virtio X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Sep 2016 01:44:56 -0000 On Thu, 22 Sep 2016 00:08:38 +0000 "Dey, Souvik" wrote: > Answers inline. > > -- > Regards, > Souvik > > -----Original Message----- > From: Stephen Hemminger [mailto:stephen@networkplumber.org] > Sent: Wednesday, September 21, 2016 7:22 PM > To: Dey, Souvik > Cc: mark.b.kavanagh@intel.com; yuanhan.liu@linux.intel.com; dev@dpdk.org > Subject: Re: [PATCH v6] net/virtio: add set_mtu in virtio > > On Wed, 21 Sep 2016 19:11:47 -0400 > Dey wrote: > > > > > + > > +#define VLAN_TAG_SIZE 4 /* 802.3ac tag (not DMA'd) */ > > + > > +static int virtio_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) { > > + struct rte_eth_dev_info dev_info; > > + uint32_t ether_hdr_len = ETHER_HDR_LEN + ETHER_CRC_LEN + VLAN_TAG_SIZE; > > + uint32_t frame_size = mtu + ether_hdr_len; > > + > > + virtio_dev_info_get(dev, &dev_info); > > + > > + if (mtu < ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen) { > > + PMD_INIT_LOG(ERR, "MTU should be between %d and %d\n", > > + ETHER_MIN_MTU, > > + (dev_info.max_rx_pktlen - ether_hdr_len)); > > + return -EINVAL; > > + } > > + return 0; > > +} > > I am fine with the general idea of this patch but: > 1. Calling virtio_dev_info_get is needlessly wasteful when all you want > is to access the max packet length. Since max_rx_pktlen is always > VIRTIO_MAX_RX_PKTLEN, please just use that. > [Dey, Souvik] I am using the virtio_dev_info_get as in future can/may support the max_rx_pktlen as a variable to be set by the application. This will keep the changes future proof. As we need to support till 65535 instead of 9728 as the linux does. Fine, then just dereference hw->rx_max_pktlen. Driver code can/should reference its own data directly. > > 2. Defining VLAN_TAG_SIZE is irrelevant if doing vlan offload. > [Dey, Souvik] vlan offload is not mandatory. Se again still have vlan being sent up to the application. In that case we need to consider the vlan length in the Ethernet size. The code needs to handle both vlan offload (or not), correctly. You are assuming the worst case here. > > 3. Virtio doesn't insert CRC, therefore CRC_LEN is irrelevant > [Dey, Souvik] I am not sure of this. Mark commented earlier to consider this length too. Mark what do you suggest ? Actually, the thing that matters is the size of the merge rx buf header, not the CRC. The patch is still buggy.