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 <pa...@mellanox.com>
> Co-authored-by: Roi Dayan <r...@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 <r...@mellanox.com>
> Acked-by: Joe Stringer <j...@ovn.org>
> Acked-by: Flavio Leitner <f...@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
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
