| Message ID | 20230421151651.3032616-4-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Improve linux QoS for exotic and fast links | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On Fri, Apr 21, 2023 at 05:16:47PM +0200, Adrian Moreno wrote: > tc uses these "rtab" tables to estimate the time (ticks) that it takes > to send a packet of different sizes. In preparation for the introduction > of 64-bit rates, add an argument to tc_put_rtab() to allow an external > 64-bit rate. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > lib/netdev-linux.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 6d5cf8114..60a0a8b2a 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -512,7 +512,8 @@ static int tc_del_qdisc(struct netdev *netdev); > static int tc_query_qdisc(const struct netdev *netdev); > > void > -tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate); > +tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate, > + uint64_t bps); Its unfortunate that there are now two parameters relating to the rate. But I think this is a case of playing the hand we've been dealt. And as I have no better idea I'm ok with this. Reviewed-by: Simon Horman <simon.horman@corigine.com>
On 4/21/23 17:16, Adrian Moreno wrote: > tc uses these "rtab" tables to estimate the time (ticks) that it takes > to send a packet of different sizes. In preparation for the introduction > of 64-bit rates, add an argument to tc_put_rtab() to allow an external > 64-bit rate. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > lib/netdev-linux.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 6d5cf8114..60a0a8b2a 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -512,7 +512,8 @@ static int tc_del_qdisc(struct netdev *netdev); > static int tc_query_qdisc(const struct netdev *netdev); > > void > -tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate); > +tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate, > + uint64_t bps); > static int tc_calc_cell_log(unsigned int mtu); > static void tc_fill_rate(struct tc_ratespec *rate, uint64_t bps, int mtu); > static int tc_calc_buffer(unsigned int Bps, int mtu, uint64_t burst_bytes); > @@ -2687,7 +2688,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police *police, > nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset, > single_action); > if (police->rate.rate) { > - tc_put_rtab(request, TCA_POLICE_RATE, &police->rate); > + tc_put_rtab(request, TCA_POLICE_RATE, &police->rate, 0); > } > if (pkts_rate) { > uint64_t pkt_burst_ticks; > @@ -2733,6 +2734,7 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t kbits_rate, > action_offset = nl_msg_start_nested(&request, TCA_MATCHALL_ACT); > nl_msg_put_act_police(&request, &pol_act, kpkts_rate * 1000, > kpkts_burst * 1000, TC_ACT_UNSPEC, false); > + > nl_msg_end_nested(&request, action_offset); > nl_msg_end_nested(&request, basic_offset); > > @@ -4665,8 +4667,8 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle, > nl_msg_put_string(&request, TCA_KIND, "htb"); > opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS); > nl_msg_put_unspec(&request, TCA_HTB_PARMS, &opt, sizeof opt); > - tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate); > - tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil); > + tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate, 0); > + tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil, 0); > nl_msg_end_nested(&request, opt_offset); > > error = tc_transact(&request, NULL); > @@ -6348,9 +6350,13 @@ tc_fill_rate(struct tc_ratespec *rate, uint64_t Bps, int mtu) > /* Appends to 'msg' an "rtab" table for the specified 'rate' as a Netlink > * attribute of the specified "type". > * > + * A 64-bit rate can be provided via 'bps'. If zero, the rate in 'rate' will > + * be used. > + * > * See tc_calc_cell_log() above for a description of "rtab"s. */ > void > -tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate) > +tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate, > + uint64_t bps) Maybe call it 'rate64' or 'rate64_bps' or something like that? Just 'bps' is not very descriptive. > { > uint32_t *rtab; > unsigned int i; > @@ -6361,7 +6367,7 @@ tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate) > if (packet_size < rate->mpu) { > packet_size = rate->mpu; > } > - rtab[i] = tc_bytes_to_ticks(rate->rate, packet_size); > + rtab[i] = tc_bytes_to_ticks(bps ? bps : rate->rate, packet_size); > } > } >
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 6d5cf8114..60a0a8b2a 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -512,7 +512,8 @@ static int tc_del_qdisc(struct netdev *netdev); static int tc_query_qdisc(const struct netdev *netdev); void -tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate); +tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate, + uint64_t bps); static int tc_calc_cell_log(unsigned int mtu); static void tc_fill_rate(struct tc_ratespec *rate, uint64_t bps, int mtu); static int tc_calc_buffer(unsigned int Bps, int mtu, uint64_t burst_bytes); @@ -2687,7 +2688,7 @@ nl_msg_put_act_police(struct ofpbuf *request, struct tc_police *police, nl_msg_act_police_start_nest(request, ++prio, &offset, &act_offset, single_action); if (police->rate.rate) { - tc_put_rtab(request, TCA_POLICE_RATE, &police->rate); + tc_put_rtab(request, TCA_POLICE_RATE, &police->rate, 0); } if (pkts_rate) { uint64_t pkt_burst_ticks; @@ -2733,6 +2734,7 @@ tc_add_matchall_policer(struct netdev *netdev, uint32_t kbits_rate, action_offset = nl_msg_start_nested(&request, TCA_MATCHALL_ACT); nl_msg_put_act_police(&request, &pol_act, kpkts_rate * 1000, kpkts_burst * 1000, TC_ACT_UNSPEC, false); + nl_msg_end_nested(&request, action_offset); nl_msg_end_nested(&request, basic_offset); @@ -4665,8 +4667,8 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle, nl_msg_put_string(&request, TCA_KIND, "htb"); opt_offset = nl_msg_start_nested(&request, TCA_OPTIONS); nl_msg_put_unspec(&request, TCA_HTB_PARMS, &opt, sizeof opt); - tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate); - tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil); + tc_put_rtab(&request, TCA_HTB_RTAB, &opt.rate, 0); + tc_put_rtab(&request, TCA_HTB_CTAB, &opt.ceil, 0); nl_msg_end_nested(&request, opt_offset); error = tc_transact(&request, NULL); @@ -6348,9 +6350,13 @@ tc_fill_rate(struct tc_ratespec *rate, uint64_t Bps, int mtu) /* Appends to 'msg' an "rtab" table for the specified 'rate' as a Netlink * attribute of the specified "type". * + * A 64-bit rate can be provided via 'bps'. If zero, the rate in 'rate' will + * be used. + * * See tc_calc_cell_log() above for a description of "rtab"s. */ void -tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate) +tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate, + uint64_t bps) { uint32_t *rtab; unsigned int i; @@ -6361,7 +6367,7 @@ tc_put_rtab(struct ofpbuf *msg, uint16_t type, const struct tc_ratespec *rate) if (packet_size < rate->mpu) { packet_size = rate->mpu; } - rtab[i] = tc_bytes_to_ticks(rate->rate, packet_size); + rtab[i] = tc_bytes_to_ticks(bps ? bps : rate->rate, packet_size); } }
tc uses these "rtab" tables to estimate the time (ticks) that it takes to send a packet of different sizes. In preparation for the introduction of 64-bit rates, add an argument to tc_put_rtab() to allow an external 64-bit rate. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- lib/netdev-linux.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)