From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60041.outbound.protection.outlook.com [40.107.6.41]) by dpdk.org (Postfix) with ESMTP id E20C1201 for ; Sun, 14 Oct 2018 13:07:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=r1q1TGpRsmr3MFPanw5uLMA37m9LxDDsA7UKumT4tM4=; b=nspSPZBKJsiugx1IVJ4zGYCLYOXWjk9cVDI9/1LwPyV0KTophQ0kh7bMb1dH8xylMO++3cBdZYCagx4Mbd0vxdyk1oT5ocKuHMVjDiqWv2P9IwYBIEf+WbYNuAy5fcZwFK0jZ1q58TE0/Y7EN+mRsZ2aM4l/IVAoZPVkFR2gltQ= Received: from DB7PR05MB4426.eurprd05.prod.outlook.com (52.134.109.15) by DB7PR05MB5383.eurprd05.prod.outlook.com (20.177.122.212) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1228.21; Sun, 14 Oct 2018 11:07:11 +0000 Received: from DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::80e:e6b:baf2:d973]) by DB7PR05MB4426.eurprd05.prod.outlook.com ([fe80::80e:e6b:baf2:d973%3]) with mapi id 15.20.1228.020; Sun, 14 Oct 2018 11:07:11 +0000 From: Shahaf Shuler To: Mordechay Haimovsky CC: "dev@dpdk.org" , Slava Ovsiienko Thread-Topic: [PATCH v2 1/2] net/mlx5: refactor TC-flow infrastructure Thread-Index: AQHUYWL0Utxkj9DMqUCkCsdPOlLAdKUebQpg Date: Sun, 14 Oct 2018 11:07:11 +0000 Message-ID: References: <1539263057-16678-1-git-send-email-motih@mellanox.com> <1539263057-16678-2-git-send-email-motih@mellanox.com> In-Reply-To: <1539263057-16678-2-git-send-email-motih@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=shahafs@mellanox.com; x-originating-ip: [31.154.10.105] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB7PR05MB5383; 6:8icmfJYmoELYItsUxtf7D/KoNFc6w2nsSbBOYsBRqSwSweulygifp2IWJlH3I0KtWVElzbl1sTOxj0DM05wT5GDOCBP0giN/jitoIxw+Vi3lkPU2ZdcLaldMXUZ54xX4fB3M3AFMdRWTMkb+sOTt+TLnEGL8d5MpPaU8YWwrfTS2VlBBXHeVEWSYtycwo89A+pX8jdnEZhp3ss5q7trQYV1oobapYSYkIm+fcnauxwNTPsbXEjoncVkHxKWnAsxBltLiIT1m9LKOpqeime+KVVt1EdV4k71kq3stv5D/Xn8b8J6OgdvO9bqLJkCTm1tNscm8OGErkATkPAYvnzaqTqEbL88fHCUTLW/WAN43Kd9CspC3TMD+acViVaDTwwmUABoQUw/JkNs2tYVMYuRoMJ6VothUpcUwQJkjYvhrj+Hc8jwdnpgD3Tqk7cMDr6wrVkXt0Sdb+gLM6gAEfyffRg==; 5:3NSYeUrIpIH2L50Y1g9jvH1cxHaPShmibmxXyPu3NyjrgJHD9Y98xPbFD4GQsqbnD96lhy0DAr/3k3MREM8KBlQaVz5q4hUEKl9/IM7reLckZQsBOuyO95PjsO6jty0RuyDUUuqrogCy0W2mgFaPoIeWC5omNgbuIdwzsv4ia/k=; 7:w8Txtf//3FxjlwwDsqR/DZT/nrVw4MQPczi4WjJ6YXVJEOvGW2+1L9l4axzherR/GXbGpPjTeX/mejDKds8cYCaBNFfTHdO+G3CuoIB//7vQcfyhH6b9l4SrIdM3HthzhYQSqRXvY+paTHN/5PZof8LyJgNr82DkSWh+CQ+gxBK/DZs2bxmGNVpPX5jAXWNeETike9P+l8e03Z50IUKK4bf2bCE0+AYy4ocPTqjVEcGMQXbUdUIfq9BmNFAoulwq x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: e871c29d-9a63-402f-d620-08d631c530a1 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:DB7PR05MB5383; x-ms-traffictypediagnostic: DB7PR05MB5383: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(269456686620040); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(3231355)(944501410)(52105095)(6055026)(149066)(150057)(6041310)(20161123562045)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(201708071742011)(7699051)(76991067); SRVR:DB7PR05MB5383; BCL:0; PCL:0; RULEID:; SRVR:DB7PR05MB5383; x-forefront-prvs: 08252193F3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(396003)(376002)(346002)(136003)(39860400002)(199004)(189003)(478600001)(66066001)(486006)(54906003)(26005)(7696005)(99286004)(68736007)(476003)(76176011)(11346002)(186003)(446003)(8936002)(8676002)(2900100001)(102836004)(6506007)(256004)(14444005)(81156014)(81166006)(71190400001)(6862004)(107886003)(4744004)(71200400001)(5250100002)(4326008)(14454004)(33656002)(25786009)(97736004)(305945005)(6436002)(53936002)(9686003)(2906002)(5660300001)(106356001)(55016002)(6246003)(316002)(6636002)(86362001)(7736002)(74316002)(229853002)(105586002)(3846002)(6116002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB7PR05MB5383; H:DB7PR05MB4426.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: rVxxtGlEWNER8IFZrWoHq8CKI8bF18MxR5h9mvUz6rg/w7eCOw5bk53qOEZjUk9IzT2vgy/xd+CJzvvv06GGbllqk8UYBLBbbqsLz4z21YKOrs+HwoQMmnLQap/1B8sksiL/7iBzr4VzDrcaKKYM+I+LeV504ldn+EgfAXYVFj05t6OAIjOuTf6sT37PhS1xpaMdu9c27OGd8jLVIIpOQmm+9oa3G4frGCzJPY92nmQHAv1nh1vFBgwYMBwav+nJdppuXakaPds83sFot6n36UqRK+VGV0BTlYm/+DDl9Pa1jUWOptRSMZzlGuGDU186vM2j5DteC8iHLJfL5BU0AsoSWdJB+LpYkOae6c6HQno= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: e871c29d-9a63-402f-d620-08d631c530a1 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Oct 2018 11:07:11.1859 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR05MB5383 Subject: Re: [dpdk-dev] [PATCH v2 1/2] net/mlx5: refactor TC-flow infrastructure 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: Sun, 14 Oct 2018 11:07:13 -0000 Hi Moty, Thursday, October 11, 2018 4:05 PM, Mordechay Haimovsky: > Subject: [PATCH v2 1/2] net/mlx5: refactor TC-flow infrastructure >=20 > This commit refactors tc_flow as a preparation to coming commits that sen= ds > different type of messages and expect differ type of replies while still = using > the same underlying routines. >=20 > Signed-off-by: Moti Haimovsky > --- > drivers/net/mlx5/mlx5.c | 18 +++---- > drivers/net/mlx5/mlx5.h | 4 +- > drivers/net/mlx5/mlx5_flow.h | 8 +-- > drivers/net/mlx5/mlx5_flow_tcf.c | 113 > ++++++++++++++++++++++++++++++--------- > 4 files changed, 102 insertions(+), 41 deletions(-) >=20 > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index > 7425e2f..20adf88 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -286,8 +286,8 @@ > close(priv->nl_socket_route); > if (priv->nl_socket_rdma >=3D 0) > close(priv->nl_socket_rdma); > - if (priv->mnl_socket) > - mlx5_flow_tcf_socket_destroy(priv->mnl_socket); > + if (priv->tcf_context) > + mlx5_flow_tcf_context_destroy(priv->tcf_context); > ret =3D mlx5_hrxq_ibv_verify(dev); > if (ret) > DRV_LOG(WARNING, "port %u some hash Rx queue still > remain", @@ -1137,8 +1137,8 @@ > claim_zero(mlx5_mac_addr_add(eth_dev, &mac, 0, 0)); > if (vf && config.vf_nl_en) > mlx5_nl_mac_addr_sync(eth_dev); > - priv->mnl_socket =3D mlx5_flow_tcf_socket_create(); > - if (!priv->mnl_socket) { > + priv->tcf_context =3D mlx5_flow_tcf_context_create(); > + if (!priv->tcf_context) { > err =3D -rte_errno; > DRV_LOG(WARNING, > "flow rules relying on switch offloads will not be" > @@ -1153,7 +1153,7 @@ > error.message =3D > "cannot retrieve network interface index"; > } else { > - err =3D mlx5_flow_tcf_init(priv->mnl_socket, ifindex, > + err =3D mlx5_flow_tcf_init(priv->tcf_context, ifindex, > &error); > } > if (err) { > @@ -1161,8 +1161,8 @@ > "flow rules relying on switch offloads will" > " not be supported: %s: %s", > error.message, strerror(rte_errno)); > - mlx5_flow_tcf_socket_destroy(priv->mnl_socket); > - priv->mnl_socket =3D NULL; > + mlx5_flow_tcf_context_destroy(priv->tcf_context); > + priv->tcf_context =3D NULL; > } > } > TAILQ_INIT(&priv->flows); > @@ -1217,8 +1217,8 @@ > close(priv->nl_socket_route); > if (priv->nl_socket_rdma >=3D 0) > close(priv->nl_socket_rdma); > - if (priv->mnl_socket) > - mlx5_flow_tcf_socket_destroy(priv->mnl_socket); > + if (priv->tcf_context) > + mlx5_flow_tcf_context_destroy(priv->tcf_context); > if (own_domain_id) > claim_zero(rte_eth_switch_domain_free(priv- > >domain_id)); > rte_free(priv); > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index > 2dec88a..d14239c 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -169,7 +169,7 @@ struct mlx5_drop { > struct mlx5_rxq_ibv *rxq; /* Verbs Rx queue. */ }; >=20 > -struct mnl_socket; > +struct mlx5_flow_tcf_context; >=20 > struct priv { > LIST_ENTRY(priv) mem_event_cb; /* Called by memory event > callback. */ @@ -236,7 +236,7 @@ struct priv { > rte_spinlock_t uar_lock[MLX5_UAR_PAGE_NUM_MAX]; > /* UAR same-page access control required in 32bit implementations. > */ #endif > - struct mnl_socket *mnl_socket; /* Libmnl socket. */ > + struct mlx5_flow_tcf_context *tcf_context; /* TC flower context. */ > }; >=20 > #define PORT_ID(priv) ((priv)->dev_data->port_id) diff --git > a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index > fee05f0..41d55e8 100644 > --- a/drivers/net/mlx5/mlx5_flow.h > +++ b/drivers/net/mlx5/mlx5_flow.h > @@ -338,9 +338,9 @@ int mlx5_flow_validate_item_vxlan_gpe(const struct > rte_flow_item *item, >=20 > /* mlx5_flow_tcf.c */ >=20 > -int mlx5_flow_tcf_init(struct mnl_socket *nl, unsigned int ifindex, > - struct rte_flow_error *error); > -struct mnl_socket *mlx5_flow_tcf_socket_create(void); > -void mlx5_flow_tcf_socket_destroy(struct mnl_socket *nl); > +int mlx5_flow_tcf_init(struct mlx5_flow_tcf_context *ctx, > + unsigned int ifindex, struct rte_flow_error *error); struct > +mlx5_flow_tcf_context *mlx5_flow_tcf_context_create(void); > +void mlx5_flow_tcf_context_destroy(struct mlx5_flow_tcf_context *ctx); >=20 > #endif /* RTE_PMD_MLX5_FLOW_H_ */ > diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c > b/drivers/net/mlx5/mlx5_flow_tcf.c > index 91f6ef6..8535a15 100644 > --- a/drivers/net/mlx5/mlx5_flow_tcf.c > +++ b/drivers/net/mlx5/mlx5_flow_tcf.c > @@ -153,6 +153,19 @@ struct tc_vlan { > #define IPV6_ADDR_LEN 16 > #endif >=20 > +/** > + * Structure for holding netlink context. > + * Note the size of the message buffer which is > MNL_SOCKET_BUFFER_SIZE. > + * Using this (8KB) buffer size ensures that netlink messages will > +never be > + * truncated. > + */ > +struct mlx5_flow_tcf_context { > + struct mnl_socket *nl; /* NETLINK_ROUTE libmnl socket. */ > + uint32_t seq; /* Message sequence number. */ I don't think the seq is needed here. See comments below.=20 > + uint32_t buf_size; /* Message buffer size. */ If the buffer is always MNL_SOCKET_BUFFER_SIZE why not statically allocate = it, like=20 uint8_t buf[MNL_SOCKET_BUFFER_SIZE]; however I don't think you need it. See comments on of the next patch in the= series. > + uint8_t *buf; /* Message buffer. */ > +}; so in fact maybe this struct is not needed at all.=20 > + > /** Empty masks for known item types. */ static const union { > struct rte_flow_item_port_id port_id; > @@ -1429,8 +1442,8 @@ struct flow_tcf_ptoi { > /** > * Send Netlink message with acknowledgment. > * > - * @param nl > - * Libmnl socket to use. > + * @param ctx > + * Flow context to use. > * @param nlh > * Message to send. This function always raises the NLM_F_ACK flag bef= ore > * sending. > @@ -1439,12 +1452,13 @@ struct flow_tcf_ptoi { > * 0 on success, a negative errno value otherwise and rte_errno is set= . > */ > static int > -flow_tcf_nl_ack(struct mnl_socket *nl, struct nlmsghdr *nlh) > +flow_tcf_nl_ack(struct mlx5_flow_tcf_context *ctx, struct nlmsghdr > +*nlh) > { > alignas(struct nlmsghdr) > uint8_t ans[mnl_nlmsg_size(sizeof(struct nlmsgerr)) + > nlh->nlmsg_len - sizeof(*nlh)]; > - uint32_t seq =3D random(); > + uint32_t seq =3D ctx->seq++; Why changing it to serially increment?=20 Netlink messages requires requests to have a unique seq id. It is more like= ly to hit conflict on the serial increment in case both port received a sim= ilar or close number. Making each request with random value has less chances for such conflict. = =20 > + struct mnl_socket *nl =3D ctx->nl; > int ret; >=20 > nlh->nlmsg_flags |=3D NLM_F_ACK; > @@ -1479,7 +1493,7 @@ struct flow_tcf_ptoi { > struct rte_flow_error *error) > { > struct priv *priv =3D dev->data->dev_private; > - struct mnl_socket *nl =3D priv->mnl_socket; > + struct mlx5_flow_tcf_context *nl =3D priv->tcf_context; > struct mlx5_flow *dev_flow; > struct nlmsghdr *nlh; >=20 > @@ -1508,7 +1522,7 @@ struct flow_tcf_ptoi { flow_tcf_remove(struct > rte_eth_dev *dev, struct rte_flow *flow) { > struct priv *priv =3D dev->data->dev_private; > - struct mnl_socket *nl =3D priv->mnl_socket; > + struct mlx5_flow_tcf_context *nl =3D priv->tcf_context; > struct mlx5_flow *dev_flow; > struct nlmsghdr *nlh; >=20 > @@ -1560,10 +1574,47 @@ struct flow_tcf_ptoi { }; >=20 > /** > - * Initialize ingress qdisc of a given network interface. > + * Create and configure a libmnl socket for Netlink flow rules. > + * > + * @return > + * A valid libmnl socket object pointer on success, NULL otherwise and > + * rte_errno is set. > + */ > +static struct mnl_socket * > +mlx5_flow_mnl_socket_create(void) > +{ > + struct mnl_socket *nl =3D mnl_socket_open(NETLINK_ROUTE); > + > + if (nl) { > + mnl_socket_setsockopt(nl, NETLINK_CAP_ACK, &(int){ 1 }, > + sizeof(int)); > + if (!mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID)) > + return nl; > + } > + rte_errno =3D errno; > + if (nl) > + mnl_socket_close(nl); > + return NULL; > +} > + > +/** > + * Destroy a libmnl socket. > * > * @param nl > * Libmnl socket of the @p NETLINK_ROUTE kind. > + */ > +static void > +mlx5_flow_mnl_socket_destroy(struct mnl_socket *nl) { > + if (nl) > + mnl_socket_close(nl); > +} > + > +/** > + * Initialize ingress qdisc of a given network interface. > + * > + * @param nl > + * Pointer to tc-flower context to use. > * @param ifindex > * Index of network interface to initialize. > * @param[out] error > @@ -1573,8 +1624,8 @@ struct flow_tcf_ptoi { > * 0 on success, a negative errno value otherwise and rte_errno is set= . > */ > int > -mlx5_flow_tcf_init(struct mnl_socket *nl, unsigned int ifindex, > - struct rte_flow_error *error) > +mlx5_flow_tcf_init(struct mlx5_flow_tcf_context *nl, > + unsigned int ifindex, struct rte_flow_error *error) > { > struct nlmsghdr *nlh; > struct tcmsg *tcm; > @@ -1616,37 +1667,47 @@ struct flow_tcf_ptoi { } >=20 > /** > - * Create and configure a libmnl socket for Netlink flow rules. > + * Create libmnl context for Netlink flow rules. > * > * @return > * A valid libmnl socket object pointer on success, NULL otherwise and > * rte_errno is set. > */ > -struct mnl_socket * > -mlx5_flow_tcf_socket_create(void) > +struct mlx5_flow_tcf_context * > +mlx5_flow_tcf_context_create(void) > { > - struct mnl_socket *nl =3D mnl_socket_open(NETLINK_ROUTE); > - > - if (nl) { > - mnl_socket_setsockopt(nl, NETLINK_CAP_ACK, &(int){ 1 }, > - sizeof(int)); > - if (!mnl_socket_bind(nl, 0, MNL_SOCKET_AUTOPID)) > - return nl; > - } > - rte_errno =3D errno; > - if (nl) > - mnl_socket_close(nl); > + struct mlx5_flow_tcf_context *ctx =3D rte_zmalloc(__func__, > + sizeof(*ctx), > + sizeof(uint32_t)); > + if (!ctx) > + goto error; > + ctx->nl =3D mlx5_flow_mnl_socket_create(); > + if (!ctx->nl) > + goto error; > + ctx->buf_size =3D MNL_SOCKET_BUFFER_SIZE; > + ctx->buf =3D rte_zmalloc(__func__, > + ctx->buf_size, sizeof(uint32_t)); > + if (!ctx->buf) > + goto error; > + ctx->seq =3D random(); > + return ctx; > +error: > + mlx5_flow_tcf_context_destroy(ctx); > return NULL; > } >=20 > /** > - * Destroy a libmnl socket. > + * Destroy a libmnl context. > * > * @param nl > * Libmnl socket of the @p NETLINK_ROUTE kind. > */ > void > -mlx5_flow_tcf_socket_destroy(struct mnl_socket *nl) > +mlx5_flow_tcf_context_destroy(struct mlx5_flow_tcf_context *ctx) > { > - mnl_socket_close(nl); > + if (!ctx) > + return; > + mlx5_flow_mnl_socket_destroy(ctx->nl); > + rte_free(ctx->buf); > + rte_free(ctx); > } > -- > 1.8.3.1