> 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?
> 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
>
