* [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
@ 2019-08-30 19:06 David Dai
2019-08-30 19:11 ` Cong Wang
0 siblings, 1 reply; 6+ messages in thread
From: David Dai @ 2019-08-30 19:06 UTC (permalink / raw)
To: jhs, xiyou.wangcong, jiri, davem, netdev, linux-kernel; +Cc: zdai, zdai
For high speed adapter like Mellanox CX-5 card, it can reach upto
100 Gbits per second bandwidth. Currently htb already supports 64bit rate
in tc utility. However police action rate and peakrate are still limited
to 32bit value (upto 32 Gbits per second). Add 2 new attributes
TCA_POLICE_RATE64 and TCA_POLICE_RATE64 in kernel for 64bit support
so that tc utility can use them for 64bit rate and peakrate value to
break the 32bit limit, and still keep the backward binary compatibility.
Tested-by: David Dai <zdai@linux.vnet.ibm.com>
Signed-off-by: David Dai <zdai@linux.vnet.ibm.com>
---
Changelog:
v1->v2:
- Move 2 attributes TCA_POLICE_RATE64 TCA_POLICE_PEAKRATE64 after
TCA_POLICE_PAD in pkt_cls.h header.
---
include/uapi/linux/pkt_cls.h | 2 ++
net/sched/act_police.c | 27 +++++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index b057aee..a6aa466 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -160,6 +160,8 @@ enum {
TCA_POLICE_RESULT,
TCA_POLICE_TM,
TCA_POLICE_PAD,
+ TCA_POLICE_RATE64,
+ TCA_POLICE_PEAKRATE64,
__TCA_POLICE_MAX
#define TCA_POLICE_RESULT TCA_POLICE_RESULT
};
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 49cec3e..425f2a3 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -40,6 +40,8 @@ static int tcf_police_walker(struct net *net, struct sk_buff *skb,
[TCA_POLICE_PEAKRATE] = { .len = TC_RTAB_SIZE },
[TCA_POLICE_AVRATE] = { .type = NLA_U32 },
[TCA_POLICE_RESULT] = { .type = NLA_U32 },
+ [TCA_POLICE_RATE64] = { .type = NLA_U64 },
+ [TCA_POLICE_PEAKRATE64] = { .type = NLA_U64 },
};
static int tcf_police_init(struct net *net, struct nlattr *nla,
@@ -58,6 +60,7 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
struct tcf_police_params *new;
bool exists = false;
u32 index;
+ u64 rate64, prate64;
if (nla == NULL)
return -EINVAL;
@@ -155,14 +158,18 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
}
if (R_tab) {
new->rate_present = true;
- psched_ratecfg_precompute(&new->rate, &R_tab->rate, 0);
+ rate64 = tb[TCA_POLICE_RATE64] ?
+ nla_get_u64(tb[TCA_POLICE_RATE64]) : 0;
+ psched_ratecfg_precompute(&new->rate, &R_tab->rate, rate64);
qdisc_put_rtab(R_tab);
} else {
new->rate_present = false;
}
if (P_tab) {
new->peak_present = true;
- psched_ratecfg_precompute(&new->peak, &P_tab->rate, 0);
+ prate64 = tb[TCA_POLICE_PEAKRATE64] ?
+ nla_get_u64(tb[TCA_POLICE_PEAKRATE64]) : 0;
+ psched_ratecfg_precompute(&new->peak, &P_tab->rate, prate64);
qdisc_put_rtab(P_tab);
} else {
new->peak_present = false;
@@ -313,10 +320,22 @@ static int tcf_police_dump(struct sk_buff *skb, struct tc_action *a,
lockdep_is_held(&police->tcf_lock));
opt.mtu = p->tcfp_mtu;
opt.burst = PSCHED_NS2TICKS(p->tcfp_burst);
- if (p->rate_present)
+ if (p->rate_present) {
psched_ratecfg_getrate(&opt.rate, &p->rate);
- if (p->peak_present)
+ if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
+ nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
+ police->params->rate.rate_bytes_ps,
+ __TCA_POLICE_MAX))
+ goto nla_put_failure;
+ }
+ if (p->peak_present) {
psched_ratecfg_getrate(&opt.peakrate, &p->peak);
+ if ((police->params->peak.rate_bytes_ps >= (1ULL << 32)) &&
+ nla_put_u64_64bit(skb, TCA_POLICE_PEAKRATE64,
+ police->params->peak.rate_bytes_ps,
+ __TCA_POLICE_MAX))
+ goto nla_put_failure;
+ }
if (nla_put(skb, TCA_POLICE_TBF, sizeof(opt), &opt))
goto nla_put_failure;
if (p->tcfp_result &&
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
2019-08-30 19:06 [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate David Dai
@ 2019-08-30 19:11 ` Cong Wang
2019-08-30 20:03 ` David Z. Dai
0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2019-08-30 19:11 UTC (permalink / raw)
To: David Dai
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, LKML, zdai
On Fri, Aug 30, 2019 at 12:06 PM David Dai <zdai@linux.vnet.ibm.com> wrote:
> - if (p->peak_present)
> + if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
> + nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
> + police->params->rate.rate_bytes_ps,
> + __TCA_POLICE_MAX))
I think the last parameter should be TCA_POLICE_PAD.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
2019-08-30 19:11 ` Cong Wang
@ 2019-08-30 20:03 ` David Z. Dai
2019-08-30 20:09 ` Cong Wang
2019-08-30 20:33 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: David Z. Dai @ 2019-08-30 20:03 UTC (permalink / raw)
To: Cong Wang
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, LKML, zdai
On Fri, 2019-08-30 at 12:11 -0700, Cong Wang wrote:
> On Fri, Aug 30, 2019 at 12:06 PM David Dai <zdai@linux.vnet.ibm.com> wrote:
> > - if (p->peak_present)
> > + if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
> > + nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
> > + police->params->rate.rate_bytes_ps,
> > + __TCA_POLICE_MAX))
>
> I think the last parameter should be TCA_POLICE_PAD.
Thanks for reviewing it!
I have the impression that last parameter num value should be larger
than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
case). This is the reason I changed the last parameter value to
__TCA_POLICE_MAX after I moved the new attributes after TC_POLICE_PAD in
pkt_cls.h header.
I rebuilt the kernel module act_police.ko by using TC_POLICE_PAD in the
4 parameter as before, I am able to set > 32bit rate and peakrate value
in tc command. It also works properly.
If the rest of community thinks I should keep using TC_POLICE_PAD in the
4th parameter too, I can change it to TC_POLICE_PAD in the next version.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
2019-08-30 20:03 ` David Z. Dai
@ 2019-08-30 20:09 ` Cong Wang
2019-08-30 20:33 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2019-08-30 20:09 UTC (permalink / raw)
To: David Z. Dai
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, LKML, zdai
On Fri, Aug 30, 2019 at 1:03 PM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
>
> On Fri, 2019-08-30 at 12:11 -0700, Cong Wang wrote:
> > On Fri, Aug 30, 2019 at 12:06 PM David Dai <zdai@linux.vnet.ibm.com> wrote:
> > > - if (p->peak_present)
> > > + if ((police->params->rate.rate_bytes_ps >= (1ULL << 32)) &&
> > > + nla_put_u64_64bit(skb, TCA_POLICE_RATE64,
> > > + police->params->rate.rate_bytes_ps,
> > > + __TCA_POLICE_MAX))
> >
> > I think the last parameter should be TCA_POLICE_PAD.
> Thanks for reviewing it!
> I have the impression that last parameter num value should be larger
> than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
Why do you have this impression?
> case). This is the reason I changed the last parameter value to
> __TCA_POLICE_MAX after I moved the new attributes after TC_POLICE_PAD in
> pkt_cls.h header.
The prototype clearly shows it must be a padding attribute:
static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype,
u64 value, int padattr)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
2019-08-30 20:03 ` David Z. Dai
2019-08-30 20:09 ` Cong Wang
@ 2019-08-30 20:33 ` David Miller
2019-08-31 0:30 ` David Z. Dai
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2019-08-30 20:33 UTC (permalink / raw)
To: zdai; +Cc: xiyou.wangcong, jhs, jiri, netdev, linux-kernel, zdai
From: "David Z. Dai" <zdai@linux.vnet.ibm.com>
Date: Fri, 30 Aug 2019 15:03:52 -0500
> I have the impression that last parameter num value should be larger
> than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
> case).
The argument in question is explicitly the "padding" value.
Please explain in detail where you got the impression that the
argument has to be larger?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
2019-08-30 20:33 ` David Miller
@ 2019-08-31 0:30 ` David Z. Dai
0 siblings, 0 replies; 6+ messages in thread
From: David Z. Dai @ 2019-08-31 0:30 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, jhs, jiri, netdev, linux-kernel, zdai
On Fri, 2019-08-30 at 13:33 -0700, David Miller wrote:
> From: "David Z. Dai" <zdai@linux.vnet.ibm.com>
> Date: Fri, 30 Aug 2019 15:03:52 -0500
>
> > I have the impression that last parameter num value should be larger
> > than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this
> > case).
>
> The argument in question is explicitly the "padding" value.
>
> Please explain in detail where you got the impression that the
> argument has to be larger?
In include/uapi/linux/pkt_sched.h header:
For HTB:
enum {
TCA_HTB_UNSPEC,
TCA_HTB_PARMS,
TCA_HTB_INIT,
TCA_HTB_CTAB,
TCA_HTB_RTAB,
TCA_HTB_DIRECT_QLEN,
TCA_HTB_RATE64, /* <--- */
TCA_HTB_CEIL64, /* <--- */
TCA_HTB_PAD, /* <--- */
__TCA_HTB_MAX,
};
/* TCA_HTB_RATE64,TCA_HTB_CEIL64 are declared *before* TCA_HTB_PAD */
For TBF:
enum {
TCA_TBF_UNSPEC,
TCA_TBF_PARMS,
TCA_TBF_RTAB,
TCA_TBF_PTAB,
TCA_TBF_RATE64, /* <--- */
TCA_TBF_PRATE64, /* <--- */
TCA_TBF_BURST,
TCA_TBF_PBURST,
TCA_TBF_PAD, /* <--- */
__TCA_TBF_MAX,
};
/* TCA_TBF_RATE64, TCA_TBF_PRATE64 are declared *before* TCA_TBF_PAD */
For HTB, in net/sched/sch_htb.c file, htb_dump_class() routine:
if ((cl->rate.rate_bytes_ps >= (1ULL << 32)) &&
nla_put_u64_64bit(skb, TCA_HTB_RATE64,
cl->rate.rate_bytes_ps,
TCA_HTB_PAD))
goto nla_put_failure;
if ((cl->ceil.rate_bytes_ps >= (1ULL << 32)) &&
nla_put_u64_64bit(skb, TCA_HTB_CEIL64,
cl->ceil.rate_bytes_ps,
TCA_HTB_PAD))
goto nla_put_failure;
For TBF, in net/sched/sch_tbf.c file, tbf_dump() routine:
if (q->rate.rate_bytes_ps >= (1ULL << 32) &&
nla_put_u64_64bit(skb, TCA_TBF_RATE64,
q->rate.rate_bytes_ps,
TCA_TBF_PAD))
goto nla_put_failure;
if (tbf_peak_present(q) &&
q->peak.rate_bytes_ps >= (1ULL << 32) &&
nla_put_u64_64bit(skb, TCA_TBF_PRATE64,
q->peak.rate_bytes_ps,
TCA_TBF_PAD))
goto nla_put_failure;
The last parameter used TCA_TBF_PAD, TCA_TBF_PAD are all declared
*after* those attributes.
I am trying to keep it consistent in police part. That's where my
impression is coming from.
Now for suggestion/comment, do you think is it better to add a new PAD
attribute in include/uapi/pkt_cls.h like this:
enum {
TCA_POLICE_UNSPEC,
TCA_POLICE_TBF,
TCA_POLICE_RATE,
TCA_POLICE_PEAKRATE,
TCA_POLICE_AVRATE,
TCA_POLICE_RESULT,
TCA_POLICE_TM,
TCA_POLICE_PAD,
TCA_POLICE_RATE64, /* <--- */
TCA_POLICE_PEAKRATE64, /* <--- */
TCA_POLICE_PAD2, /* <--- new PAD */
__TCA_POLICE_MAX
#define TCA_POLICE_RESULT TCA_POLICE_RESULT
#};
Then use this TCA_POLICE_PAD2 as the last parameter in
nla_put_u64_64bit()?
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-31 0:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 19:06 [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate David Dai
2019-08-30 19:11 ` Cong Wang
2019-08-30 20:03 ` David Z. Dai
2019-08-30 20:09 ` Cong Wang
2019-08-30 20:33 ` David Miller
2019-08-31 0:30 ` David Z. Dai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).