From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ilya Matveychikov Subject: Re: [PATCH] mbuf: fix for incomplete nb_segs types change Date: Mon, 13 Nov 2017 14:18:55 +0400 Message-ID: <2FAC1B54-ACCA-439E-BE58-10007DB35E56@gmail.com> References: <04EF52DF-6CE8-4EF4-96AA-711184F8379F@gmail.com> <20171110135643.24107-1-matvejchikov@gmail.com> <20171113101017.cdk5e7kjmwwdju36@platinum> Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org To: Olivier MATZ Return-path: Received: from mail-lf0-f45.google.com (mail-lf0-f45.google.com [209.85.215.45]) by dpdk.org (Postfix) with ESMTP id AD7DC1AEE8 for ; Mon, 13 Nov 2017 11:18:59 +0100 (CET) Received: by mail-lf0-f45.google.com with SMTP id 73so3339652lfu.10 for ; Mon, 13 Nov 2017 02:18:59 -0800 (PST) In-Reply-To: <20171113101017.cdk5e7kjmwwdju36@platinum> 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 Nov 13, 2017, at 2:10 PM, Olivier MATZ = wrote: >=20 > Hi Ilya, >=20 > 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. >>=20 >> Fixes: 97cb466d65c9 ("mbuf: use 2 bytes for port and nb segments") >> Cc: olivier.matz@6wind.com >>=20 >> Signed-off-by: Ilya V. Matveychikov >=20 > Thanks for this fix. >=20 > I did quick search for other references: >=20 > $ git grep seg | grep -i int8 > app/test-pmd/config.c: tx_pkt_nb_segs =3D (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 =3D 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 =3D= (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 =3D (uint8_t = *)sg_addr; > drivers/net/dpaa2/dpaa2_rxtx.c: next_seg->buf_addr =3D = (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 =3D 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 =3D= { > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: uint8_t *dseg; > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg =3D (uint8_t = *)(wqe + 1); > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg =3D= (uint8_t *) > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: uint8_t *dseg; > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg =3D (uint8_t *)(wqe + 1); > drivers/net/mlx5/mlx5_rxtx_vec_neon.h: dseg =3D (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 =3D 1; > drivers/net/qede/qede_rxtx.c: uint8_t nb_segs =3D 0; > drivers/net/sfc/base/ef10_nvram.c: if ((rc =3D = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > drivers/net/sfc/base/ef10_nvram.c: if ((rc =3D = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > drivers/net/sfc/base/ef10_nvram.c: if ((rc =3D = tlv_init_cursor_from_size(&cursor, (uint8_t *)seg_data, > drivers/net/sfc/base/ef10_nvram.c: if ((rc =3D = 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 =3D = (uint8_t)(pkt->nb_segs + 1); > lib/librte_mbuf/rte_mbuf.h: uint8_t nseg; > lib/librte_mbuf/rte_mbuf.h: head->nb_segs =3D = (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) = { >=20 > 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. >=20 Something already fixed by: 6c293ffd63b3f61a95d8c492d007160874e7a057 4c20622a95087b42ddb403406d3877a9dc3d731b But I didn=E2=80=99t 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=E2=80=99s better to apply this patch = separately without integration with drivers fixes and stuff like that. Ilya.=