> On Jul 12, 2018, at 3:46 AM, Adrien Mazarguil <adrien.mazarg...@6wind.com> > wrote: > > On Wed, Jul 11, 2018 at 05:17:09PM -0700, Yongseok Koh wrote: >> On Wed, Jun 27, 2018 at 08:08:10PM +0200, Adrien Mazarguil wrote: >>> With mlx5, unlike normal flow rules implemented through Verbs for traffic >>> emitted and received by the application, those targeting different logical >>> ports of the device (VF representors for instance) are offloaded at the >>> switch level and must be configured through Netlink (TC interface). >>> >>> This patch adds preliminary support to manage such flow rules through the >>> flow API (rte_flow). >>> >>> Instead of rewriting tons of Netlink helpers and as previously suggested by >>> Stephen [1], this patch introduces a new dependency to libmnl [2] >>> (LGPL-2.1) when compiling mlx5. >>> >>> [1] >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dpdk.org%2Farchives%2Fdev%2F2018-March%2F092676.html&data=02%7C01%7Cyskoh%40mellanox.com%7C1250093eca0c4ad6d9f008d5dc58fbb4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636657197116524482&sdata=JrAyzK1s3JG5CnuquNcA7XRN4d2WYtHUi1KXyloGdvA%3D&reserved=0 >>> [2] >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fnetfilter.org%2Fprojects%2Flibmnl%2F&data=02%7C01%7Cyskoh%40mellanox.com%7C1250093eca0c4ad6d9f008d5dc58fbb4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636657197116524482&sdata=yLYa0NzsTyE62BHDCZDoDah31snt6w4Coq47pY913Oo%3D&reserved=0 >>> >>> Signed-off-by: Adrien Mazarguil <adrien.mazarg...@6wind.com> > <snip> >>> diff --git a/drivers/net/mlx5/mlx5_nl_flow.c >>> b/drivers/net/mlx5/mlx5_nl_flow.c >>> new file mode 100644 >>> index 000000000..7a8683b03 >>> --- /dev/null >>> +++ b/drivers/net/mlx5/mlx5_nl_flow.c >>> @@ -0,0 +1,139 @@ >>> +/* SPDX-License-Identifier: BSD-3-Clause >>> + * Copyright 2018 6WIND S.A. >>> + * Copyright 2018 Mellanox Technologies, Ltd >>> + */ >>> + >>> +#include <errno.h> >>> +#include <libmnl/libmnl.h> >>> +#include <linux/netlink.h> >>> +#include <linux/pkt_sched.h> >>> +#include <linux/rtnetlink.h> >>> +#include <stdalign.h> >>> +#include <stddef.h> >>> +#include <stdint.h> >>> +#include <stdlib.h> >>> +#include <sys/socket.h> >>> + >>> +#include <rte_errno.h> >>> +#include <rte_flow.h> >>> + >>> +#include "mlx5.h" >>> + >>> +/** >>> + * Send Netlink message with acknowledgment. >>> + * >>> + * @param nl >>> + * Libmnl socket to use. >>> + * @param nlh >>> + * Message to send. This function always raises the NLM_F_ACK flag before >>> + * sending. >>> + * >>> + * @return >>> + * 0 on success, a negative errno value otherwise and rte_errno is set. >>> + */ >>> +static int >>> +mlx5_nl_flow_nl_ack(struct mnl_socket *nl, struct nlmsghdr *nlh) >>> +{ >>> + alignas(struct nlmsghdr) >>> + uint8_t ans[MNL_SOCKET_BUFFER_SIZE]; >> >> There are total 3 of this buffer. On a certain host having large pagesize, >> this >> can be 8kB * 3 = 24kB. This is not a gigantic buffer but as all the functions >> here are sequentially accessed, how about having just one global buffer >> instead? > > All right it's not ideal, I opted for simplicity though. This is a generic > ack function. When NETLINK_CAP_ACK is not supported (note: this was made > optional for v2, some systems do not support it), an ack consumes a bit more > space than the original message, which may itself be huge, and failure to > receive acks is deemed fatal. > > Its callers are mlx5_nl_flow_init(), called once per device during > initialization, and mlx5_nl_flow_create/destroy(), called for each > created/removed flow rule. > > These last two are called often but do not put their own buffer on the > stack, they reuse previously generated messages from the heap. > > So to improve stack consumption a bit, what I can do is size this buffer > according to nlh->nlmsg_len + extra room for ack header, yet still allocate > it locally since it would be a pain otherwise. Callers may not want their > own buffers to be overwritten with useless acks.
I like this approach. Thanks, Yongseok