RE: [patch iproute2 v6 2/3] tc: Add -bs option to batch mode

[Chris Mi]

> -Original Message-
> From: Marcelo Ricardo Leitner [mailto:marcelo.leit...@gmail.com]
> Sent: Saturday, January 6, 2018 3:15 AM
> To: David Ahern <dsah...@gmail.com>
> Cc: Chris Mi <chr...@mellanox.com>; netdev@vger.kernel.org;
> gerlitz...@gmail.com; step...@networkplumber.org
> Subject: Re: [patch iproute2 v6 2/3] tc: Add -bs option to batch mode
> 
> On Fri, Jan 05, 2018 at 11:15:59AM -0700, David Ahern wrote:
> > On 1/4/18 12:34 AM, Chris Mi wrote:
> > > Currently in tc batch mode, only one command is read from the batch
> > > file and sent to kernel to process. With this support, we can
> > > accumulate several commands before sending to kernel.
> > >
> > > Now it only works for the following successive rules, 1. filter add
> > > 2. filter delete 3. actions add 4. actions delete
> > >
> > > Otherwise, the batch size is still 1.
> > >
> > > Signed-off-by: Chris Mi <chr...@mellanox.com>
> > > ---
> > >  tc/m_action.c  |  93 ++--
> > >  tc/tc.c|  96 +++--
> > >  tc/tc_common.h |   8 +++-
> > >  tc/tc_filter.c | 132
> > > -
> > >  4 files changed, 252 insertions(+), 77 deletions(-)
> > >
> > > diff --git a/tc/m_action.c b/tc/m_action.c index fc422364..cf5cc95d
> > > 100644
> > > --- a/tc/m_action.c
> > > +++ b/tc/m_action.c
> > > @@ -23,6 +23,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >
> > >  #include "utils.h"
> > >  #include "tc_common.h"
> > > @@ -546,40 +547,86 @@ bad_val:
> > >   return ret;
> > >  }
> > >
> > > +typedef struct {
> > > + struct nlmsghdr n;
> > > + struct tcamsg   t;
> > > + charbuf[MAX_MSG];
> > > +} tc_action_req;
> > > +
> > > +static tc_action_req *action_reqs;
> > > +static struct iovec msg_iov[MSG_IOV_MAX];
> > > +
> > > +void free_action_reqs(void)
> > > +{
> > > + free(action_reqs);
> > > +}
> > > +
> > > +static tc_action_req *get_action_req(int batch_size, int index) {
> > > + tc_action_req *req;
> > > +
> > > + if (action_reqs == NULL) {
> > > + action_reqs = malloc(batch_size * sizeof (tc_action_req));
> > > + if (action_reqs == NULL)
> > > + return NULL;
> > > + }
> > > + req = _reqs[index];
> > > + memset(req, 0, sizeof (*req));
> > > +
> > > + return req;
> > > +}
> > > +
> > >  static int tc_action_modify(int cmd, unsigned int flags,
> > > - int *argc_p, char ***argv_p)
> > > + int *argc_p, char ***argv_p,
> > > + int batch_size, int index, bool send)
> > >  {
> > > - int argc = *argc_p;
> > > + struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> > > + struct iovec *iov = _iov[index];
> > >   char **argv = *argv_p;
> > > - int ret = 0;
> > > - struct {
> > > - struct nlmsghdr n;
> > > - struct tcamsg   t;
> > > - charbuf[MAX_MSG];
> > > - } req = {
> > > - .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
> > > - .n.nlmsg_flags = NLM_F_REQUEST | flags,
> > > - .n.nlmsg_type = cmd,
> > > - .t.tca_family = AF_UNSPEC,
> > > + struct msghdr msg = {
> > > + .msg_name = ,
> > > + .msg_namelen = sizeof(nladdr),
> > > + .msg_iov = msg_iov,
> > > + .msg_iovlen = index + 1,
> > >   };
> > > - struct rtattr *tail = NLMSG_TAIL();
> > > + struct rtattr *tail;
> > > + tc_action_req *req;
> > > + int argc = *argc_p;
> > > + int ret = 0;
> > > +
> > > + req = get_action_req(batch_size, index);
> > > + if (req == NULL) {
> > > + fprintf(stderr, "get_action_req error: not enough buffer\n");
> > > + return -ENOMEM;
> > > + }
> > > + req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
> > > + req->n.nlmsg_flags = NLM_F_REQUEST | flags;
> > > + req->n.nlmsg_type = cmd;
> > > + req->t.tca_family = AF_UNSPEC;
> > > + tail = NLM

Re: [patch iproute2 v6 2/3] tc: Add -bs option to batch mode

[Marcelo Ricardo Leitner]

On Fri, Jan 05, 2018 at 11:15:59AM -0700, David Ahern wrote:
> On 1/4/18 12:34 AM, Chris Mi wrote:
> > Currently in tc batch mode, only one command is read from the batch
> > file and sent to kernel to process. With this support, we can accumulate
> > several commands before sending to kernel.
> > 
> > Now it only works for the following successive rules,
> > 1. filter add
> > 2. filter delete
> > 3. actions add
> > 4. actions delete
> > 
> > Otherwise, the batch size is still 1.
> > 
> > Signed-off-by: Chris Mi 
> > ---
> >  tc/m_action.c  |  93 ++--
> >  tc/tc.c|  96 +++--
> >  tc/tc_common.h |   8 +++-
> >  tc/tc_filter.c | 132 
> > -
> >  4 files changed, 252 insertions(+), 77 deletions(-)
> > 
> > diff --git a/tc/m_action.c b/tc/m_action.c
> > index fc422364..cf5cc95d 100644
> > --- a/tc/m_action.c
> > +++ b/tc/m_action.c
> > @@ -23,6 +23,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include "utils.h"
> >  #include "tc_common.h"
> > @@ -546,40 +547,86 @@ bad_val:
> > return ret;
> >  }
> >  
> > +typedef struct {
> > +   struct nlmsghdr n;
> > +   struct tcamsg   t;
> > +   charbuf[MAX_MSG];
> > +} tc_action_req;
> > +
> > +static tc_action_req *action_reqs;
> > +static struct iovec msg_iov[MSG_IOV_MAX];
> > +
> > +void free_action_reqs(void)
> > +{
> > +   free(action_reqs);
> > +}
> > +
> > +static tc_action_req *get_action_req(int batch_size, int index)
> > +{
> > +   tc_action_req *req;
> > +
> > +   if (action_reqs == NULL) {
> > +   action_reqs = malloc(batch_size * sizeof (tc_action_req));
> > +   if (action_reqs == NULL)
> > +   return NULL;
> > +   }
> > +   req = _reqs[index];
> > +   memset(req, 0, sizeof (*req));
> > +
> > +   return req;
> > +}
> > +
> >  static int tc_action_modify(int cmd, unsigned int flags,
> > -   int *argc_p, char ***argv_p)
> > +   int *argc_p, char ***argv_p,
> > +   int batch_size, int index, bool send)
> >  {
> > -   int argc = *argc_p;
> > +   struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> > +   struct iovec *iov = _iov[index];
> > char **argv = *argv_p;
> > -   int ret = 0;
> > -   struct {
> > -   struct nlmsghdr n;
> > -   struct tcamsg   t;
> > -   charbuf[MAX_MSG];
> > -   } req = {
> > -   .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
> > -   .n.nlmsg_flags = NLM_F_REQUEST | flags,
> > -   .n.nlmsg_type = cmd,
> > -   .t.tca_family = AF_UNSPEC,
> > +   struct msghdr msg = {
> > +   .msg_name = ,
> > +   .msg_namelen = sizeof(nladdr),
> > +   .msg_iov = msg_iov,
> > +   .msg_iovlen = index + 1,
> > };
> > -   struct rtattr *tail = NLMSG_TAIL();
> > +   struct rtattr *tail;
> > +   tc_action_req *req;
> > +   int argc = *argc_p;
> > +   int ret = 0;
> > +
> > +   req = get_action_req(batch_size, index);
> > +   if (req == NULL) {
> > +   fprintf(stderr, "get_action_req error: not enough buffer\n");
> > +   return -ENOMEM;
> > +   }
> > +   req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
> > +   req->n.nlmsg_flags = NLM_F_REQUEST | flags;
> > +   req->n.nlmsg_type = cmd;
> > +   req->t.tca_family = AF_UNSPEC;
> > +   tail = NLMSG_TAIL(>n);
> >  
> > argc -= 1;
> > argv += 1;
> > -   if (parse_action(, , TCA_ACT_TAB, )) {
> > +   if (parse_action(, , TCA_ACT_TAB, >n)) {
> > fprintf(stderr, "Illegal \"action\"\n");
> > return -1;
> > }
> > -   tail->rta_len = (void *) NLMSG_TAIL() - (void *) tail;
> > +   tail->rta_len = (void *) NLMSG_TAIL(>n) - (void *) tail;
> >  
> > -   if (rtnl_talk(, , NULL) < 0) {
> > +   *argc_p = argc;
> > +   *argv_p = argv;
> > +
> > +   iov->iov_base = >n;
> > +   iov->iov_len = req->n.nlmsg_len;
> > +
> > +   if (!send)
> > +   return 0;
> > +
> > +   if (rtnl_talk_msg(, , NULL) < 0) {
> > fprintf(stderr, "We have an error talking to the kernel\n");
> > ret = -1;
> > }
> >  
> > -   *argc_p = argc;
> > -   *argv_p = argv;
> > -
> > return ret;
> >  }
> >  
> > @@ -679,7 +726,7 @@ bad_val:
> > return ret;
> >  }
> >  
> > -int do_action(int argc, char **argv)
> > +int do_action(int argc, char **argv, int batch_size, int index, bool send)
> >  {
> >  
> > int ret = 0;
> > @@ -689,12 +736,14 @@ int do_action(int argc, char **argv)
> > if (matches(*argv, "add") == 0) {
> > ret =  tc_action_modify(RTM_NEWACTION,
> > NLM_F_EXCL | NLM_F_CREATE,
> > -   , );
> > +   , , batch_size,
> > +   

Re: [patch iproute2 v6 2/3] tc: Add -bs option to batch mode

[David Ahern]

On 1/4/18 12:34 AM, Chris Mi wrote:
> Currently in tc batch mode, only one command is read from the batch
> file and sent to kernel to process. With this support, we can accumulate
> several commands before sending to kernel.
> 
> Now it only works for the following successive rules,
> 1. filter add
> 2. filter delete
> 3. actions add
> 4. actions delete
> 
> Otherwise, the batch size is still 1.
> 
> Signed-off-by: Chris Mi 
> ---
>  tc/m_action.c  |  93 ++--
>  tc/tc.c|  96 +++--
>  tc/tc_common.h |   8 +++-
>  tc/tc_filter.c | 132 
> -
>  4 files changed, 252 insertions(+), 77 deletions(-)
> 
> diff --git a/tc/m_action.c b/tc/m_action.c
> index fc422364..cf5cc95d 100644
> --- a/tc/m_action.c
> +++ b/tc/m_action.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "utils.h"
>  #include "tc_common.h"
> @@ -546,40 +547,86 @@ bad_val:
>   return ret;
>  }
>  
> +typedef struct {
> + struct nlmsghdr n;
> + struct tcamsg   t;
> + charbuf[MAX_MSG];
> +} tc_action_req;
> +
> +static tc_action_req *action_reqs;
> +static struct iovec msg_iov[MSG_IOV_MAX];
> +
> +void free_action_reqs(void)
> +{
> + free(action_reqs);
> +}
> +
> +static tc_action_req *get_action_req(int batch_size, int index)
> +{
> + tc_action_req *req;
> +
> + if (action_reqs == NULL) {
> + action_reqs = malloc(batch_size * sizeof (tc_action_req));
> + if (action_reqs == NULL)
> + return NULL;
> + }
> + req = _reqs[index];
> + memset(req, 0, sizeof (*req));
> +
> + return req;
> +}
> +
>  static int tc_action_modify(int cmd, unsigned int flags,
> - int *argc_p, char ***argv_p)
> + int *argc_p, char ***argv_p,
> + int batch_size, int index, bool send)
>  {
> - int argc = *argc_p;
> + struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> + struct iovec *iov = _iov[index];
>   char **argv = *argv_p;
> - int ret = 0;
> - struct {
> - struct nlmsghdr n;
> - struct tcamsg   t;
> - charbuf[MAX_MSG];
> - } req = {
> - .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
> - .n.nlmsg_flags = NLM_F_REQUEST | flags,
> - .n.nlmsg_type = cmd,
> - .t.tca_family = AF_UNSPEC,
> + struct msghdr msg = {
> + .msg_name = ,
> + .msg_namelen = sizeof(nladdr),
> + .msg_iov = msg_iov,
> + .msg_iovlen = index + 1,
>   };
> - struct rtattr *tail = NLMSG_TAIL();
> + struct rtattr *tail;
> + tc_action_req *req;
> + int argc = *argc_p;
> + int ret = 0;
> +
> + req = get_action_req(batch_size, index);
> + if (req == NULL) {
> + fprintf(stderr, "get_action_req error: not enough buffer\n");
> + return -ENOMEM;
> + }
> + req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
> + req->n.nlmsg_flags = NLM_F_REQUEST | flags;
> + req->n.nlmsg_type = cmd;
> + req->t.tca_family = AF_UNSPEC;
> + tail = NLMSG_TAIL(>n);
>  
>   argc -= 1;
>   argv += 1;
> - if (parse_action(, , TCA_ACT_TAB, )) {
> + if (parse_action(, , TCA_ACT_TAB, >n)) {
>   fprintf(stderr, "Illegal \"action\"\n");
>   return -1;
>   }
> - tail->rta_len = (void *) NLMSG_TAIL() - (void *) tail;
> + tail->rta_len = (void *) NLMSG_TAIL(>n) - (void *) tail;
>  
> - if (rtnl_talk(, , NULL) < 0) {
> + *argc_p = argc;
> + *argv_p = argv;
> +
> + iov->iov_base = >n;
> + iov->iov_len = req->n.nlmsg_len;
> +
> + if (!send)
> + return 0;
> +
> + if (rtnl_talk_msg(, , NULL) < 0) {
>   fprintf(stderr, "We have an error talking to the kernel\n");
>   ret = -1;
>   }
>  
> - *argc_p = argc;
> - *argv_p = argv;
> -
>   return ret;
>  }
>  
> @@ -679,7 +726,7 @@ bad_val:
>   return ret;
>  }
>  
> -int do_action(int argc, char **argv)
> +int do_action(int argc, char **argv, int batch_size, int index, bool send)
>  {
>  
>   int ret = 0;
> @@ -689,12 +736,14 @@ int do_action(int argc, char **argv)
>   if (matches(*argv, "add") == 0) {
>   ret =  tc_action_modify(RTM_NEWACTION,
>   NLM_F_EXCL | NLM_F_CREATE,
> - , );
> + , , batch_size,
> + index, send);
>   } else if (matches(*argv, "change") == 0 ||
> matches(*argv, "replace") == 0) {
>   ret = 

Re: [patch iproute2 v6 2/3] tc: Add -bs option to batch mode

[Marcelo Ricardo Leitner]

On Thu, Jan 04, 2018 at 04:34:53PM +0900, Chris Mi wrote:
> Currently in tc batch mode, only one command is read from the batch
> file and sent to kernel to process. With this support, we can accumulate
> several commands before sending to kernel.
> 
> Now it only works for the following successive rules,
> 1. filter add
> 2. filter delete
> 3. actions add
> 4. actions delete
> 
> Otherwise, the batch size is still 1.
> 
> Signed-off-by: Chris Mi 
> ---
>  tc/m_action.c  |  93 ++--
>  tc/tc.c|  96 +++--
>  tc/tc_common.h |   8 +++-
>  tc/tc_filter.c | 132 
> -
>  4 files changed, 252 insertions(+), 77 deletions(-)
> 
> diff --git a/tc/m_action.c b/tc/m_action.c
> index fc422364..cf5cc95d 100644
> --- a/tc/m_action.c
> +++ b/tc/m_action.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "utils.h"
>  #include "tc_common.h"
> @@ -546,40 +547,86 @@ bad_val:
>   return ret;
>  }
>  
> +typedef struct {
> + struct nlmsghdr n;
> + struct tcamsg   t;
> + charbuf[MAX_MSG];
> +} tc_action_req;
> +
> +static tc_action_req *action_reqs;
> +static struct iovec msg_iov[MSG_IOV_MAX];
> +
> +void free_action_reqs(void)
> +{
> + free(action_reqs);
> +}
> +
> +static tc_action_req *get_action_req(int batch_size, int index)
> +{
> + tc_action_req *req;
> +
> + if (action_reqs == NULL) {
> + action_reqs = malloc(batch_size * sizeof (tc_action_req));
> + if (action_reqs == NULL)
> + return NULL;
> + }
> + req = _reqs[index];
> + memset(req, 0, sizeof (*req));
> +
> + return req;
> +}
> +
>  static int tc_action_modify(int cmd, unsigned int flags,
> - int *argc_p, char ***argv_p)
> + int *argc_p, char ***argv_p,
> + int batch_size, int index, bool send)
>  {
> - int argc = *argc_p;
> + struct sockaddr_nl nladdr = { .nl_family = AF_NETLINK };
> + struct iovec *iov = _iov[index];
>   char **argv = *argv_p;
> - int ret = 0;
> - struct {
> - struct nlmsghdr n;
> - struct tcamsg   t;
> - charbuf[MAX_MSG];
> - } req = {
> - .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg)),
> - .n.nlmsg_flags = NLM_F_REQUEST | flags,
> - .n.nlmsg_type = cmd,
> - .t.tca_family = AF_UNSPEC,
> + struct msghdr msg = {
> + .msg_name = ,
> + .msg_namelen = sizeof(nladdr),
> + .msg_iov = msg_iov,
> + .msg_iovlen = index + 1,
>   };
> - struct rtattr *tail = NLMSG_TAIL();
> + struct rtattr *tail;
> + tc_action_req *req;
> + int argc = *argc_p;
> + int ret = 0;
> +
> + req = get_action_req(batch_size, index);
> + if (req == NULL) {
> + fprintf(stderr, "get_action_req error: not enough buffer\n");
> + return -ENOMEM;
> + }
> + req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct tcamsg));
> + req->n.nlmsg_flags = NLM_F_REQUEST | flags;
> + req->n.nlmsg_type = cmd;
> + req->t.tca_family = AF_UNSPEC;
> + tail = NLMSG_TAIL(>n);
>  
>   argc -= 1;
>   argv += 1;
> - if (parse_action(, , TCA_ACT_TAB, )) {
> + if (parse_action(, , TCA_ACT_TAB, >n)) {
>   fprintf(stderr, "Illegal \"action\"\n");
>   return -1;
>   }
> - tail->rta_len = (void *) NLMSG_TAIL() - (void *) tail;
> + tail->rta_len = (void *) NLMSG_TAIL(>n) - (void *) tail;
>  
> - if (rtnl_talk(, , NULL) < 0) {
> + *argc_p = argc;
> + *argv_p = argv;
> +
> + iov->iov_base = >n;
> + iov->iov_len = req->n.nlmsg_len;
> +
> + if (!send)
> + return 0;
> +
> + if (rtnl_talk_msg(, , NULL) < 0) {
>   fprintf(stderr, "We have an error talking to the kernel\n");
>   ret = -1;
>   }
>  
> - *argc_p = argc;
> - *argv_p = argv;
> -
>   return ret;
>  }
>  
> @@ -679,7 +726,7 @@ bad_val:
>   return ret;
>  }
>  
> -int do_action(int argc, char **argv)
> +int do_action(int argc, char **argv, int batch_size, int index, bool send)
>  {
>  
>   int ret = 0;
> @@ -689,12 +736,14 @@ int do_action(int argc, char **argv)
>   if (matches(*argv, "add") == 0) {
>   ret =  tc_action_modify(RTM_NEWACTION,
>   NLM_F_EXCL | NLM_F_CREATE,
> - , );
> + , , batch_size,
> + index, send);
>   } else if (matches(*argv, "change") == 0 ||
> matches(*argv, "replace") == 0) {
>   ret =