diff mbox

[ovs-dev,V10,01/33] netdev-linux: Refactor two tc functions

Message ID 1496922410-36853-2-git-send-email-roid@mellanox.com
State Superseded
Headers show

Commit Message

Roi Dayan June 8, 2017, 11:46 a.m. UTC 
Refactor tc_make_request and tc_add_del_ingress_qdisc to accept
ifindex instead of netdev struct.
We later want to move those outside netdev-linux module to be
used by other modules.
This patch doesn't change any functionality.

Signed-off-by: Paul Blakey <paulb@mellanox.com>
Co-authored-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Roi Dayan <roid@mellanox.com>
Acked-by: Joe Stringer <joe@ovn.org>
Acked-by: Flavio Leitner <fbl@sysclose.org>
---
 lib/netdev-linux.c | 91 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 55 insertions(+), 36 deletions(-)

Comments

Simon Horman June 8, 2017, 2:24 p.m. UTC | #1 
On Thu, Jun 08, 2017 at 02:46:18PM +0300, Roi Dayan wrote:
> Refactor tc_make_request and tc_add_del_ingress_qdisc to accept
> ifindex instead of netdev struct.
> We later want to move those outside netdev-linux module to be
> used by other modules.
> This patch doesn't change any functionality.
> 
> Signed-off-by: Paul Blakey <paulb@mellanox.com>
> Co-authored-by: Roi Dayan <roid@mellanox.com>

Hi Roi,

as your name appears in the From field (as the author) I think
that Paul's name rather than yours should be in the Co-authored-by tag.
If you agree please consider responding to this email with the correct tag.

> Signed-off-by: Roi Dayan <roid@mellanox.com>
> Acked-by: Joe Stringer <joe@ovn.org>
> Acked-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  lib/netdev-linux.c | 91 +++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 55 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 1b88775..d794453 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -442,10 +442,14 @@ static unsigned int tc_ticks_to_bytes(unsigned int rate, unsigned int ticks);
>  static unsigned int tc_bytes_to_ticks(unsigned int rate, unsigned int size);
>  static unsigned int tc_buffer_per_jiffy(unsigned int rate);
>  
> -static struct tcmsg *tc_make_request(const struct netdev *, int type,
> +static struct tcmsg *tc_make_request(int ifindex, int type,
>                                       unsigned int flags, struct ofpbuf *);
> +static struct tcmsg *netdev_linux_tc_make_request(const struct netdev *,
> +                                                  int type,
> +                                                  unsigned int flags,
> +                                                  struct ofpbuf *);
>  static int tc_transact(struct ofpbuf *request, struct ofpbuf **replyp);
> -static int tc_add_del_ingress_qdisc(struct netdev *netdev, bool add);
> +static int tc_add_del_ingress_qdisc(int ifindex, bool add);
>  static int tc_add_policer(struct netdev *,
>                            uint32_t kbits_rate, uint32_t kbits_burst);
>  
> @@ -2089,6 +2093,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>  {
>      struct netdev_linux *netdev = netdev_linux_cast(netdev_);
>      const char *netdev_name = netdev_get_name(netdev_);
> +    int ifindex;
>      int error;
>  
>      kbits_burst = (!kbits_rate ? 0       /* Force to 0 if no rate specified. */
> @@ -2106,9 +2111,14 @@ netdev_linux_set_policing(struct netdev *netdev_,
>          netdev->cache_valid &= ~VALID_POLICING;
>      }
>  
> +    error = get_ifindex(netdev_, &ifindex);
> +    if (error) {
> +        goto out;
> +    }
> +
>      COVERAGE_INC(netdev_set_policing);
>      /* Remove any existing ingress qdisc. */
> -    error = tc_add_del_ingress_qdisc(netdev_, false);
> +    error = tc_add_del_ingress_qdisc(ifindex, false);
>      if (error) {
>          VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
>                       netdev_name, ovs_strerror(error));
> @@ -2116,7 +2126,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>      }
>  
>      if (kbits_rate) {
> -        error = tc_add_del_ingress_qdisc(netdev_, true);
> +        error = tc_add_del_ingress_qdisc(ifindex, true);
>          if (error) {
>              VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
>                           netdev_name, ovs_strerror(error));
> @@ -2385,7 +2395,7 @@ start_queue_dump(const struct netdev *netdev, struct queue_dump_state *state)
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
>  
> -    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
>      if (!tcmsg) {
>          return false;
>      }
> @@ -2944,8 +2954,8 @@ codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
>  
>      tc_del_qdisc(netdev);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -3162,8 +3172,8 @@ fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
>  
>      tc_del_qdisc(netdev);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -3386,8 +3396,8 @@ sfq_setup_qdisc__(struct netdev *netdev, uint32_t quantum, uint32_t perturb)
>  
>      tc_del_qdisc(netdev);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -3573,8 +3583,8 @@ htb_setup_qdisc__(struct netdev *netdev)
>  
>      tc_del_qdisc(netdev);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -3627,7 +3637,8 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle,
>      opt.cbuffer = tc_calc_buffer(opt.ceil.rate, mtu, class->burst);
>      opt.prio = class->priority;
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
> +                                         &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -4236,8 +4247,8 @@ hfsc_setup_qdisc__(struct netdev * netdev)
>  
>      tc_del_qdisc(netdev);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>  
>      if (!tcmsg) {
>          return ENODEV;
> @@ -4269,7 +4280,8 @@ hfsc_setup_class__(struct netdev *netdev, unsigned int handle,
>      struct ofpbuf request;
>      struct tc_service_curve min, max;
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
> +                                         &request);
>  
>      if (!tcmsg) {
>          return ENODEV;
> @@ -4667,17 +4679,10 @@ tc_get_minor(unsigned int handle)
>  }
>  
>  static struct tcmsg *
> -tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
> +tc_make_request(int ifindex, int type, unsigned int flags,
>                  struct ofpbuf *request)
>  {
>      struct tcmsg *tcmsg;
> -    int ifindex;
> -    int error;
> -
> -    error = get_ifindex(netdev, &ifindex);
> -    if (error) {
> -        return NULL;
> -    }
>  
>      ofpbuf_init(request, 512);
>      nl_msg_put_nlmsghdr(request, sizeof *tcmsg, type, NLM_F_REQUEST | flags);
> @@ -4690,6 +4695,21 @@ tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
>      return tcmsg;
>  }
>  
> +static struct tcmsg *
> +netdev_linux_tc_make_request(const struct netdev *netdev, int type,
> +                             unsigned int flags, struct ofpbuf *request)
> +{
> +    int ifindex;
> +    int error;
> +
> +    error = get_ifindex(netdev, &ifindex);
> +    if (error) {
> +        return NULL;
> +    }
> +
> +    return tc_make_request(ifindex, type, flags, request);
> +}
> +
>  static int
>  tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
>  {
> @@ -4713,7 +4733,7 @@ tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
>   * Returns 0 if successful, otherwise a positive errno value.
>   */
>  static int
> -tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
> +tc_add_del_ingress_qdisc(int ifindex, bool add)
>  {
>      struct ofpbuf request;
>      struct tcmsg *tcmsg;
> @@ -4721,10 +4741,7 @@ tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
>      int type = add ? RTM_NEWQDISC : RTM_DELQDISC;
>      int flags = add ? NLM_F_EXCL | NLM_F_CREATE : 0;
>  
> -    tcmsg = tc_make_request(netdev, type, flags, &request);
> -    if (!tcmsg) {
> -        return ENODEV;
> -    }
> +    tcmsg = tc_make_request(ifindex, type, flags, &request);
>      tcmsg->tcm_handle = tc_make_handle(0xffff, 0);
>      tcmsg->tcm_parent = TC_H_INGRESS;
>      nl_msg_put_string(&request, TCA_KIND, "ingress");
> @@ -4783,8 +4800,8 @@ tc_add_policer(struct netdev *netdev,
>      tc_police.burst = tc_bytes_to_ticks(
>          tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
>  
> -    tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTFILTER,
> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -5049,7 +5066,8 @@ tc_query_class(const struct netdev *netdev,
>      struct tcmsg *tcmsg;
>      int error;
>  
> -    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO,
> +                                         &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -5075,7 +5093,7 @@ tc_delete_class(const struct netdev *netdev, unsigned int handle)
>      struct tcmsg *tcmsg;
>      int error;
>  
> -    tcmsg = tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -5101,7 +5119,7 @@ tc_del_qdisc(struct netdev *netdev_)
>      struct tcmsg *tcmsg;
>      int error;
>  
> -    tcmsg = tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> @@ -5182,7 +5200,8 @@ tc_query_qdisc(const struct netdev *netdev_)
>       * in such a case we get no response at all from the kernel (!) if a
>       * builtin qdisc is in use (which is later caught by "!error &&
>       * !qdisc->size"). */
> -    tcmsg = tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO, &request);
> +    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO,
> +                                         &request);
>      if (!tcmsg) {
>          return ENODEV;
>      }
> -- 
> 2.7.4
>
Roi Dayan June 12, 2017, 2:45 p.m. UTC | #2 
On 08/06/2017 17:24, Simon Horman wrote:
> On Thu, Jun 08, 2017 at 02:46:18PM +0300, Roi Dayan wrote:
>> Refactor tc_make_request and tc_add_del_ingress_qdisc to accept
>> ifindex instead of netdev struct.
>> We later want to move those outside netdev-linux module to be
>> used by other modules.
>> This patch doesn't change any functionality.
>>
>> Signed-off-by: Paul Blakey <paulb@mellanox.com>
>> Co-authored-by: Roi Dayan <roid@mellanox.com>
>
> Hi Roi,
>
> as your name appears in the From field (as the author) I think
> that Paul's name rather than yours should be in the Co-authored-by tag.
> If you agree please consider responding to this email with the correct tag.
>

hi,

ok. you mean reply like this

Signed-off-by: Roi Dayan <roid@mellanox.com>
Co-authored-by: Paul Blakey <paulb@mellanox.com>


Thanks,
Roi



>> Signed-off-by: Roi Dayan <roid@mellanox.com>
>> Acked-by: Joe Stringer <joe@ovn.org>
>> Acked-by: Flavio Leitner <fbl@sysclose.org>
>> ---
>>  lib/netdev-linux.c | 91 +++++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 55 insertions(+), 36 deletions(-)
>>
>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>> index 1b88775..d794453 100644
>> --- a/lib/netdev-linux.c
>> +++ b/lib/netdev-linux.c
>> @@ -442,10 +442,14 @@ static unsigned int tc_ticks_to_bytes(unsigned int rate, unsigned int ticks);
>>  static unsigned int tc_bytes_to_ticks(unsigned int rate, unsigned int size);
>>  static unsigned int tc_buffer_per_jiffy(unsigned int rate);
>>
>> -static struct tcmsg *tc_make_request(const struct netdev *, int type,
>> +static struct tcmsg *tc_make_request(int ifindex, int type,
>>                                       unsigned int flags, struct ofpbuf *);
>> +static struct tcmsg *netdev_linux_tc_make_request(const struct netdev *,
>> +                                                  int type,
>> +                                                  unsigned int flags,
>> +                                                  struct ofpbuf *);
>>  static int tc_transact(struct ofpbuf *request, struct ofpbuf **replyp);
>> -static int tc_add_del_ingress_qdisc(struct netdev *netdev, bool add);
>> +static int tc_add_del_ingress_qdisc(int ifindex, bool add);
>>  static int tc_add_policer(struct netdev *,
>>                            uint32_t kbits_rate, uint32_t kbits_burst);
>>
>> @@ -2089,6 +2093,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>>  {
>>      struct netdev_linux *netdev = netdev_linux_cast(netdev_);
>>      const char *netdev_name = netdev_get_name(netdev_);
>> +    int ifindex;
>>      int error;
>>
>>      kbits_burst = (!kbits_rate ? 0       /* Force to 0 if no rate specified. */
>> @@ -2106,9 +2111,14 @@ netdev_linux_set_policing(struct netdev *netdev_,
>>          netdev->cache_valid &= ~VALID_POLICING;
>>      }
>>
>> +    error = get_ifindex(netdev_, &ifindex);
>> +    if (error) {
>> +        goto out;
>> +    }
>> +
>>      COVERAGE_INC(netdev_set_policing);
>>      /* Remove any existing ingress qdisc. */
>> -    error = tc_add_del_ingress_qdisc(netdev_, false);
>> +    error = tc_add_del_ingress_qdisc(ifindex, false);
>>      if (error) {
>>          VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
>>                       netdev_name, ovs_strerror(error));
>> @@ -2116,7 +2126,7 @@ netdev_linux_set_policing(struct netdev *netdev_,
>>      }
>>
>>      if (kbits_rate) {
>> -        error = tc_add_del_ingress_qdisc(netdev_, true);
>> +        error = tc_add_del_ingress_qdisc(ifindex, true);
>>          if (error) {
>>              VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
>>                           netdev_name, ovs_strerror(error));
>> @@ -2385,7 +2395,7 @@ start_queue_dump(const struct netdev *netdev, struct queue_dump_state *state)
>>      struct ofpbuf request;
>>      struct tcmsg *tcmsg;
>>
>> -    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
>>      if (!tcmsg) {
>>          return false;
>>      }
>> @@ -2944,8 +2954,8 @@ codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
>>
>>      tc_del_qdisc(netdev);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -3162,8 +3172,8 @@ fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
>>
>>      tc_del_qdisc(netdev);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -3386,8 +3396,8 @@ sfq_setup_qdisc__(struct netdev *netdev, uint32_t quantum, uint32_t perturb)
>>
>>      tc_del_qdisc(netdev);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -3573,8 +3583,8 @@ htb_setup_qdisc__(struct netdev *netdev)
>>
>>      tc_del_qdisc(netdev);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -3627,7 +3637,8 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle,
>>      opt.cbuffer = tc_calc_buffer(opt.ceil.rate, mtu, class->burst);
>>      opt.prio = class->priority;
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
>> +                                         &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -4236,8 +4247,8 @@ hfsc_setup_qdisc__(struct netdev * netdev)
>>
>>      tc_del_qdisc(netdev);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>
>>      if (!tcmsg) {
>>          return ENODEV;
>> @@ -4269,7 +4280,8 @@ hfsc_setup_class__(struct netdev *netdev, unsigned int handle,
>>      struct ofpbuf request;
>>      struct tc_service_curve min, max;
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
>> +                                         &request);
>>
>>      if (!tcmsg) {
>>          return ENODEV;
>> @@ -4667,17 +4679,10 @@ tc_get_minor(unsigned int handle)
>>  }
>>
>>  static struct tcmsg *
>> -tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
>> +tc_make_request(int ifindex, int type, unsigned int flags,
>>                  struct ofpbuf *request)
>>  {
>>      struct tcmsg *tcmsg;
>> -    int ifindex;
>> -    int error;
>> -
>> -    error = get_ifindex(netdev, &ifindex);
>> -    if (error) {
>> -        return NULL;
>> -    }
>>
>>      ofpbuf_init(request, 512);
>>      nl_msg_put_nlmsghdr(request, sizeof *tcmsg, type, NLM_F_REQUEST | flags);
>> @@ -4690,6 +4695,21 @@ tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
>>      return tcmsg;
>>  }
>>
>> +static struct tcmsg *
>> +netdev_linux_tc_make_request(const struct netdev *netdev, int type,
>> +                             unsigned int flags, struct ofpbuf *request)
>> +{
>> +    int ifindex;
>> +    int error;
>> +
>> +    error = get_ifindex(netdev, &ifindex);
>> +    if (error) {
>> +        return NULL;
>> +    }
>> +
>> +    return tc_make_request(ifindex, type, flags, request);
>> +}
>> +
>>  static int
>>  tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
>>  {
>> @@ -4713,7 +4733,7 @@ tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
>>   * Returns 0 if successful, otherwise a positive errno value.
>>   */
>>  static int
>> -tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
>> +tc_add_del_ingress_qdisc(int ifindex, bool add)
>>  {
>>      struct ofpbuf request;
>>      struct tcmsg *tcmsg;
>> @@ -4721,10 +4741,7 @@ tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
>>      int type = add ? RTM_NEWQDISC : RTM_DELQDISC;
>>      int flags = add ? NLM_F_EXCL | NLM_F_CREATE : 0;
>>
>> -    tcmsg = tc_make_request(netdev, type, flags, &request);
>> -    if (!tcmsg) {
>> -        return ENODEV;
>> -    }
>> +    tcmsg = tc_make_request(ifindex, type, flags, &request);
>>      tcmsg->tcm_handle = tc_make_handle(0xffff, 0);
>>      tcmsg->tcm_parent = TC_H_INGRESS;
>>      nl_msg_put_string(&request, TCA_KIND, "ingress");
>> @@ -4783,8 +4800,8 @@ tc_add_policer(struct netdev *netdev,
>>      tc_police.burst = tc_bytes_to_ticks(
>>          tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
>>
>> -    tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
>> -                            NLM_F_EXCL | NLM_F_CREATE, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTFILTER,
>> +                                         NLM_F_EXCL | NLM_F_CREATE, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -5049,7 +5066,8 @@ tc_query_class(const struct netdev *netdev,
>>      struct tcmsg *tcmsg;
>>      int error;
>>
>> -    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO,
>> +                                         &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -5075,7 +5093,7 @@ tc_delete_class(const struct netdev *netdev, unsigned int handle)
>>      struct tcmsg *tcmsg;
>>      int error;
>>
>> -    tcmsg = tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -5101,7 +5119,7 @@ tc_del_qdisc(struct netdev *netdev_)
>>      struct tcmsg *tcmsg;
>>      int error;
>>
>> -    tcmsg = tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> @@ -5182,7 +5200,8 @@ tc_query_qdisc(const struct netdev *netdev_)
>>       * in such a case we get no response at all from the kernel (!) if a
>>       * builtin qdisc is in use (which is later caught by "!error &&
>>       * !qdisc->size"). */
>> -    tcmsg = tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO, &request);
>> +    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO,
>> +                                         &request);
>>      if (!tcmsg) {
>>          return ENODEV;
>>      }
>> --
>> 2.7.4
>>
diff mbox

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 1b88775..d794453 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -442,10 +442,14 @@  static unsigned int tc_ticks_to_bytes(unsigned int rate, unsigned int ticks);
 static unsigned int tc_bytes_to_ticks(unsigned int rate, unsigned int size);
 static unsigned int tc_buffer_per_jiffy(unsigned int rate);
 
-static struct tcmsg *tc_make_request(const struct netdev *, int type,
+static struct tcmsg *tc_make_request(int ifindex, int type,
                                      unsigned int flags, struct ofpbuf *);
+static struct tcmsg *netdev_linux_tc_make_request(const struct netdev *,
+                                                  int type,
+                                                  unsigned int flags,
+                                                  struct ofpbuf *);
 static int tc_transact(struct ofpbuf *request, struct ofpbuf **replyp);
-static int tc_add_del_ingress_qdisc(struct netdev *netdev, bool add);
+static int tc_add_del_ingress_qdisc(int ifindex, bool add);
 static int tc_add_policer(struct netdev *,
                           uint32_t kbits_rate, uint32_t kbits_burst);
 
@@ -2089,6 +2093,7 @@  netdev_linux_set_policing(struct netdev *netdev_,
 {
     struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     const char *netdev_name = netdev_get_name(netdev_);
+    int ifindex;
     int error;
 
     kbits_burst = (!kbits_rate ? 0       /* Force to 0 if no rate specified. */
@@ -2106,9 +2111,14 @@  netdev_linux_set_policing(struct netdev *netdev_,
         netdev->cache_valid &= ~VALID_POLICING;
     }
 
+    error = get_ifindex(netdev_, &ifindex);
+    if (error) {
+        goto out;
+    }
+
     COVERAGE_INC(netdev_set_policing);
     /* Remove any existing ingress qdisc. */
-    error = tc_add_del_ingress_qdisc(netdev_, false);
+    error = tc_add_del_ingress_qdisc(ifindex, false);
     if (error) {
         VLOG_WARN_RL(&rl, "%s: removing policing failed: %s",
                      netdev_name, ovs_strerror(error));
@@ -2116,7 +2126,7 @@  netdev_linux_set_policing(struct netdev *netdev_,
     }
 
     if (kbits_rate) {
-        error = tc_add_del_ingress_qdisc(netdev_, true);
+        error = tc_add_del_ingress_qdisc(ifindex, true);
         if (error) {
             VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
                          netdev_name, ovs_strerror(error));
@@ -2385,7 +2395,7 @@  start_queue_dump(const struct netdev *netdev, struct queue_dump_state *state)
     struct ofpbuf request;
     struct tcmsg *tcmsg;
 
-    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, 0, &request);
     if (!tcmsg) {
         return false;
     }
@@ -2944,8 +2954,8 @@  codel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
 
     tc_del_qdisc(netdev);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -3162,8 +3172,8 @@  fqcodel_setup_qdisc__(struct netdev *netdev, uint32_t target, uint32_t limit,
 
     tc_del_qdisc(netdev);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -3386,8 +3396,8 @@  sfq_setup_qdisc__(struct netdev *netdev, uint32_t quantum, uint32_t perturb)
 
     tc_del_qdisc(netdev);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -3573,8 +3583,8 @@  htb_setup_qdisc__(struct netdev *netdev)
 
     tc_del_qdisc(netdev);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -3627,7 +3637,8 @@  htb_setup_class__(struct netdev *netdev, unsigned int handle,
     opt.cbuffer = tc_calc_buffer(opt.ceil.rate, mtu, class->burst);
     opt.prio = class->priority;
 
-    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
+                                         &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -4236,8 +4247,8 @@  hfsc_setup_qdisc__(struct netdev * netdev)
 
     tc_del_qdisc(netdev);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWQDISC,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWQDISC,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
 
     if (!tcmsg) {
         return ENODEV;
@@ -4269,7 +4280,8 @@  hfsc_setup_class__(struct netdev *netdev, unsigned int handle,
     struct ofpbuf request;
     struct tc_service_curve min, max;
 
-    tcmsg = tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTCLASS, NLM_F_CREATE,
+                                         &request);
 
     if (!tcmsg) {
         return ENODEV;
@@ -4667,17 +4679,10 @@  tc_get_minor(unsigned int handle)
 }
 
 static struct tcmsg *
-tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
+tc_make_request(int ifindex, int type, unsigned int flags,
                 struct ofpbuf *request)
 {
     struct tcmsg *tcmsg;
-    int ifindex;
-    int error;
-
-    error = get_ifindex(netdev, &ifindex);
-    if (error) {
-        return NULL;
-    }
 
     ofpbuf_init(request, 512);
     nl_msg_put_nlmsghdr(request, sizeof *tcmsg, type, NLM_F_REQUEST | flags);
@@ -4690,6 +4695,21 @@  tc_make_request(const struct netdev *netdev, int type, unsigned int flags,
     return tcmsg;
 }
 
+static struct tcmsg *
+netdev_linux_tc_make_request(const struct netdev *netdev, int type,
+                             unsigned int flags, struct ofpbuf *request)
+{
+    int ifindex;
+    int error;
+
+    error = get_ifindex(netdev, &ifindex);
+    if (error) {
+        return NULL;
+    }
+
+    return tc_make_request(ifindex, type, flags, request);
+}
+
 static int
 tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
 {
@@ -4713,7 +4733,7 @@  tc_transact(struct ofpbuf *request, struct ofpbuf **replyp)
  * Returns 0 if successful, otherwise a positive errno value.
  */
 static int
-tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
+tc_add_del_ingress_qdisc(int ifindex, bool add)
 {
     struct ofpbuf request;
     struct tcmsg *tcmsg;
@@ -4721,10 +4741,7 @@  tc_add_del_ingress_qdisc(struct netdev *netdev, bool add)
     int type = add ? RTM_NEWQDISC : RTM_DELQDISC;
     int flags = add ? NLM_F_EXCL | NLM_F_CREATE : 0;
 
-    tcmsg = tc_make_request(netdev, type, flags, &request);
-    if (!tcmsg) {
-        return ENODEV;
-    }
+    tcmsg = tc_make_request(ifindex, type, flags, &request);
     tcmsg->tcm_handle = tc_make_handle(0xffff, 0);
     tcmsg->tcm_parent = TC_H_INGRESS;
     nl_msg_put_string(&request, TCA_KIND, "ingress");
@@ -4783,8 +4800,8 @@  tc_add_policer(struct netdev *netdev,
     tc_police.burst = tc_bytes_to_ticks(
         tc_police.rate.rate, MIN(UINT32_MAX / 1024, kbits_burst) * 1024 / 8);
 
-    tcmsg = tc_make_request(netdev, RTM_NEWTFILTER,
-                            NLM_F_EXCL | NLM_F_CREATE, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_NEWTFILTER,
+                                         NLM_F_EXCL | NLM_F_CREATE, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -5049,7 +5066,8 @@  tc_query_class(const struct netdev *netdev,
     struct tcmsg *tcmsg;
     int error;
 
-    tcmsg = tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_GETTCLASS, NLM_F_ECHO,
+                                         &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -5075,7 +5093,7 @@  tc_delete_class(const struct netdev *netdev, unsigned int handle)
     struct tcmsg *tcmsg;
     int error;
 
-    tcmsg = tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev, RTM_DELTCLASS, 0, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -5101,7 +5119,7 @@  tc_del_qdisc(struct netdev *netdev_)
     struct tcmsg *tcmsg;
     int error;
 
-    tcmsg = tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_DELQDISC, 0, &request);
     if (!tcmsg) {
         return ENODEV;
     }
@@ -5182,7 +5200,8 @@  tc_query_qdisc(const struct netdev *netdev_)
      * in such a case we get no response at all from the kernel (!) if a
      * builtin qdisc is in use (which is later caught by "!error &&
      * !qdisc->size"). */
-    tcmsg = tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO, &request);
+    tcmsg = netdev_linux_tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO,
+                                         &request);
     if (!tcmsg) {
         return ENODEV;
     }