> -----Original Message-----
> From: Yongseok Koh
> Sent: Sunday, November 11, 2018 13:42
> To: Slava Ovsiienko <viachesl...@mellanox.com>
> Cc: Shahaf Shuler <shah...@mellanox.com>; dev@dpdk.org
> Subject: Re: [PATCH 3/3] net/mlx5: fix rule cleanup Netlink command sending
>
>
> > On Nov 10, 2018, at 1:59 AM, Slava Ovsiienko
> <viachesl...@mellanox.com> wrote:
> >
> > The VXLAN related rule cleanup routine queries and gathers all
> > existing local IP and neigh rules into buffer list. One buffer may
> > contain multiple rule deletetion commands and is prepared to send into
> > Netlink as single message. But, if error occurs for some deletion
> > commands in the buffer, the multiple ACK message with errors can be
> > send back by the kernel. It breaks the Netlink communication sequence
> > numbers, because we expect only one ACK message and it smashes out
> > futher Netlik communication.
>
> Just curious.
> Is parsing the multiple ack msgs more complex than sending commands one
> by one?
We are in the midst of send query/get dump process. We can't send
another request and wait ack for it - we are receiving the dump. Possible,
it can be done via creation one more Netlink socket, but I'm not sure the
requests are not queued by kernel. So - the simplest way - gather dump
and then send commands.
PS. Actually I have refactored gathering/sending, we need to gather
parameters only, not build entire commands in callbacks, but this patch
is not tested yet and too large as for simple fix.
WBR,
Slava
>
> > The workaround of this problem is to send rule deletion commands from
> > buffer in one-by-one fashion and get ACK message for every command
> > sent. We do not expect too may rules preexist, so there should not be
> > critical performance degradation at VXLAN outer interface
> > initialization.
> >
> > Fixes: f420f03d6772 ("net/mlx5: add E-switch VXLAN rule cleanup
> > routines")
> >
> > Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> > ---
>
> Acked-by: Yongseok Koh <ys...@mellanox.com>
>
> Thanks
>
> > drivers/net/mlx5/mlx5_flow_tcf.c | 58
> > +++++++++++++++++-----------------------
> > 1 file changed, 24 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> > b/drivers/net/mlx5/mlx5_flow_tcf.c
> > index bba8aed..21eb99e 100644
> > --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> > +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> > @@ -3847,30 +3847,6 @@ struct tcf_nlcb_context { }
> >
> > /**
> > - * Set NLM_F_ACK flags in the last netlink command in buffer.
> > - * Only last command in the buffer will be acked by system.
> > - *
> > - * @param[in, out] buf
> > - * Pointer to buffer with netlink commands.
> > - */
> > -static void
> > -flow_tcf_setack_nlcmd(struct tcf_nlcb_buf *buf) -{
> > - struct nlmsghdr *nlh;
> > - uint32_t size = 0;
> > -
> > - assert(buf->size);
> > - do {
> > - nlh = (struct nlmsghdr *)&buf->msg[size];
> > - size += NLMSG_ALIGN(nlh->nlmsg_len);
> > - if (size >= buf->size) {
> > - nlh->nlmsg_flags |= NLM_F_ACK;
> > - break;
> > - }
> > - } while (true);
> > -}
> > -
> > -/**
> > * Send the buffers with prepared netlink commands. Scans the list and
> > * sends all found buffers. Buffers are sent and freed anyway in order
> > * to prevent memory leakage if some every message in received packet.
> > @@ -3888,21 +3864,35 @@ struct tcf_nlcb_context {
> > flow_tcf_send_nlcmd(struct mlx5_flow_tcf_context *tcf,
> > struct tcf_nlcb_context *ctx)
> > {
> > - struct tcf_nlcb_buf *bc, *bn;
> > - struct nlmsghdr *nlh;
> > + struct tcf_nlcb_buf *bc = LIST_FIRST(&ctx->nlbuf);
> > int ret = 0;
> >
> > - bc = LIST_FIRST(&ctx->nlbuf);
> > while (bc) {
> > + struct tcf_nlcb_buf *bn = LIST_NEXT(bc, next);
> > + struct nlmsghdr *nlh;
> > + uint32_t msg = 0;
> > int rc;
> >
> > - bn = LIST_NEXT(bc, next);
> > - if (bc->size) {
> > - flow_tcf_setack_nlcmd(bc);
> > - nlh = (struct nlmsghdr *)&bc->msg;
> > - rc = flow_tcf_nl_ack(tcf, nlh, bc->size, NULL, NULL);
> > - if (rc && !ret)
> > - ret = rc;
> > + while (msg < bc->size) {
> > + /*
> > + * Send Netlink commands from buffer in one by one
> > + * fashion. If we send multiple rule deletion
> commands
> > + * in one Netlink message and some error occurs it
> may
> > + * cause multiple ACK error messages and break
> sequence
> > + * numbers of Netlink communication, because we
> expect
> > + * the only one ACK reply.
> > + */
> > + assert((bc->size - msg) >= sizeof(struct nlmsghdr));
> > + nlh = (struct nlmsghdr *)&bc->msg[msg];
> > + assert((bc->size - msg) >= nlh->nlmsg_len);
> > + msg += nlh->nlmsg_len;
> > + rc = flow_tcf_nl_ack(tcf, nlh, 0, NULL, NULL);
> > + if (rc) {
> > + DRV_LOG(WARNING,
> > + "netlink: cleanup error %d", rc);
> > + if (!ret)
> > + ret = rc;
> > + }
> > }
> > rte_free(bc);
> > bc = bn;
> > --
> > 1.8.3.1
> >
