From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 554E02A66 for ; Mon, 13 Nov 2017 11:33:37 +0100 (CET) Received: from lfbn-1-6068-189.w90-110.abo.wanadoo.fr ([90.110.3.189] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1eEC9U-0004cN-Rz; Mon, 13 Nov 2017 11:39:42 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 13 Nov 2017 11:33:29 +0100 Date: Mon, 13 Nov 2017 11:33:29 +0100 From: Olivier MATZ To: Ilya Matveychikov Cc: dev@dpdk.org Message-ID: <20171113103328.bcxyswa3toxemht5@platinum> References: <04EF52DF-6CE8-4EF4-96AA-711184F8379F@gmail.com> <20171110135643.24107-1-matvejchikov@gmail.com> <20171113101017.cdk5e7kjmwwdju36@platinum> <2FAC1B54-ACCA-439E-BE58-10007DB35E56@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2FAC1B54-ACCA-439E-BE58-10007DB35E56@gmail.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] mbuf: fix for incomplete nb_segs types change 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: , X-List-Received-Date: Mon, 13 Nov 2017 10:33:37 -0000 On Mon, Nov 13, 2017 at 02:18:55PM +0400, Ilya Matveychikov wrote: > > > On Nov 13, 2017, at 2:10 PM, Olivier MATZ wrote: > > > > Hi Ilya, > > > > On Fri, Nov 10, 2017 at 04:56:43PM +0300, Ilya V. Matveychikov wrote: > >> Update types of variables to correspond to nb_segs type change from > >> uint8_t to uint16_t. > >> > >> Fixes: 97cb466d65c9 ("mbuf: use 2 bytes for port and nb segments") > >> Cc: olivier.matz@6wind.com > >> > >> Signed-off-by: Ilya V. Matveychikov > > > > Thanks for this fix. > > > > I did quick search for other references: > > > > $ git grep seg | grep -i int8 > > app/test-pmd/config.c: tx_pkt_nb_segs = (uint8_t) nb_segs; > > app/test-pmd/csumonly.c: uint16_t seglen[], uint8_t nb_seg) > > app/test-pmd/testpmd.c:uint8_t tx_pkt_nb_segs = 1; /**< Number of segments in TXONLY packets */ > > app/test-pmd/testpmd.h:extern uint8_t tx_pkt_nb_segs; /**< Number of segments in TX packets */ > > doc/guides/sample_app_ug/ipv4_multicast.rst: hdr->pkt.nb_segs = (uint8_t)(pkt->pkt.nb_segs + 1); > > drivers/bus/dpaa/rte_dpaa_bus.h: return (uint8_t *)(memseg[i].addr) + > > drivers/net/ark/ark_ethdev_rx.c: uint8_t segments; > > drivers/net/avp/rte_avp_common.h: uint8_t nb_segs; /**< Number of segments */ > > drivers/net/bnx2x/bnx2x.c:bnx2x_igu_ack_sb(struct bnx2x_softc *sc, uint8_t igu_sb_id, uint8_t segment, > > drivers/net/bnx2x/bnx2x.c: uint8_t segment; > > drivers/net/bnx2x/bnx2x.h:bnx2x_igu_ack_sb_gen(struct bnx2x_softc *sc, uint8_t segment, > > drivers/net/bnx2x/ecore_hsi.h: uint8_t igu_seg_id; > > drivers/net/bnx2x/ecore_hsi.h: uint8_t igu_seg_id; > > drivers/net/bnx2x/ecore_hsi.h: uint8_t igu_seg_id /* segment id of the IGU */; > > drivers/net/bnx2x/ecore_hsi.h: uint8_t igu_seg_id /* segment id of the IGU */; > > drivers/net/bnxt/hsi_struct_def_dpdk.h: uint8_t tpa_segs; > > drivers/net/dpaa2/dpaa2_rxtx.c: first_seg->buf_addr = (uint8_t *)sg_addr; > > drivers/net/dpaa2/dpaa2_rxtx.c: next_seg->buf_addr = (uint8_t *)sg_addr; > > drivers/net/i40e/i40e_rxtx.c: * m->nb_segs is uint8_t, so nb_segs is always less than > > drivers/net/ixgbe/ixgbe_rxtx.c: int8_t i, nb_segs = m->nb_segs; > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h:txq_wr_dseg_v(struct mlx5_txq_data *txq, uint8_t *dseg, > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: const uint8x16_t dseg_shuf_m = { > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: uint8_t *dseg; > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg = (uint8_t *)(wqe + 1); > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg = (uint8_t *) > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: uint8_t *dseg; > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg = (uint8_t *)(wqe + 1); > > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg = (uint8_t *)txq->wqes; > > drivers/net/qede/qede_rxtx.c: uint8_t num_segs, uint16_t pkt_len) > > drivers/net/qede/qede_rxtx.c: uint8_t num_segs = 1; > > drivers/net/qede/qede_rxtx.c: uint8_t nb_segs = 0; > > drivers/net/sfc/base/ef10_nvram.c: if ((rc = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > > drivers/net/sfc/base/ef10_nvram.c: if ((rc = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > > drivers/net/sfc/base/ef10_nvram.c: if ((rc = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > > drivers/net/sfc/base/ef10_nvram.c: if ((rc = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > > drivers/net/szedata2/rte_eth_szedata2.c: uint8_t mbuf_segs; > > drivers/net/vmxnet3/base/vmxnet3_defs.h: uint8 segCnt; /* Number of aggregated packets */ > > examples/ipv4_multicast/main.c: hdr->nb_segs = (uint8_t)(pkt->nb_segs + 1); > > lib/librte_mbuf/rte_mbuf.h: uint8_t nseg; > > lib/librte_mbuf/rte_mbuf.h: head->nb_segs = (uint8_t)(head->nb_segs + tail->nb_segs); > > lib/librte_pdump/rte_pdump.c: uint8_t nseg; > > test/test/packet_burst_generator.c: uint8_t pkt_len, uint8_t nb_pkt_segs) > > test/test/packet_burst_generator.c: int nb_pkt_per_burst, uint8_t pkt_len, uint8_t nb_pkt_segs) > > test/test/packet_burst_generator.h: uint8_t pkt_len, uint8_t nb_pkt_segs); > > test/test/packet_burst_generator.h: int nb_pkt_per_burst, uint8_t pkt_len, uint8_t nb_pkt_segs); > > test/test/test_cryptodev.h: int nb_segs, uint8_t pattern) { > > > > I think the ones in ixgbe, szedata and examples/ipv4_multicast/main.c could be > > fixed without risk. > > Some don't look critical for now (ex: testpmd), but could be integrated in this > > patch too. > > > > Something already fixed by: > 6c293ffd63b3f61a95d8c492d007160874e7a057 > 4c20622a95087b42ddb403406d3877a9dc3d731b You are right, sorry, I didn't update my workspace since rc3. > But I didn’t try to fix it everywhere in drivers as it may depends of the particular hardware - does it support more that uint8_t queues or not. So, I agree with you that maintainers should have a look on that change. > > Also, not sure that testpmd need to have more that 255 segments of 16k possible. And my opinion that it’s better to apply this patch separately without integration with drivers fixes and stuff like that. Agree. Acked-by: Olivier Matz