[ovs-dev] [PATCH V10 01/33] netdev-linux: Refactor two tc functions
Roi Dayan
roid at mellanox.com
Mon Jun 12 14:45:56 UTC 2017
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 at mellanox.com>
>> Co-authored-by: Roi Dayan <roid at 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 at mellanox.com>
Co-authored-by: Paul Blakey <paulb at mellanox.com>
Thanks,
Roi
>> Signed-off-by: Roi Dayan <roid at mellanox.com>
>> Acked-by: Joe Stringer <joe at ovn.org>
>> Acked-by: Flavio Leitner <fbl at 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
>>
More information about the dev
mailing list