From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-x22a.google.com (mail-oi0-x22a.google.com [IPv6:2607:f8b0:4003:c06::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 90BC93B260; Thu, 5 May 2016 11:30:33 -0400 (EDT) Received: by mail-oi0-x22a.google.com with SMTP id x19so106461563oix.2; Thu, 05 May 2016 08:30:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-transfer-encoding; bh=2giXvCuBc2M65poN226tnvDxx1ydtNtle6wk6hllagY=; b=G0OpQQNdUZwgpLkaKhi5yrboh2aaaFlt2YXwdPQwEI09aDLXjblaVmumRekE2z7dHd By+mTWNWh1KX15z1LSUWSp/05r3uQ+6H4bsJCI+aSB2PnkWA/B2cbZtFXginv9ifWD1u 5M0Sf5haVjpDGQIBS8BkC/H9WpfrbmrYi0nqvJbvef2cLNDg8n2QasAQYVqftcxJsJG5 q4zyGTcCr6zrHJosmC8msv35FKu+c9vwg9a8sOYH7UPWhAfC9CoaP+D/xmogsHRieDBL I6a7XjBvtKZcCQQFFrey3AcHrqyC8BuVhkapdXXHmvitG3XXh1mtzTr/PqJbdu9+eJb9 5StA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-transfer-encoding; bh=2giXvCuBc2M65poN226tnvDxx1ydtNtle6wk6hllagY=; b=ZKsTZbwRNBX8eeJAC3bYyE6uRMwMd91aGSxaMBoM7u1AegTBkaB6pZebSwwj98L5JF 7tlzTt6dLEr2v5PtsFQ+30dqBNLGue2WR8WyuOuO6O59P914cgO71KRyGZ8ae6EvE9U3 xVAKkZQ6vbOP+piEhYADxhqdIW0Eb0LnS183NL1w3O7hFBKxlMSXCZdp8PQur1np7Dw9 WacPfWe+DatH9SPChkwEz2oKkC0SUHMvcsA5F28P5JnuOi58guPuJNj7sPISxgxgakPe FDYPPjjPQs0xgvA9DQVu5psae3MWhMWnoTzzfojKlsv01BScZ8G22/b473V5cnJ0xnQP 5MaQ== X-Gm-Message-State: AOPr4FXLq+S3WNXRy6gwGZ4yGyI8c63vym/qev427UG+eXdqXiaDWhmYgashvPgIUL4Nqi23jiywVE35EWOdnQ== MIME-Version: 1.0 X-Received: by 10.202.216.68 with SMTP id p65mr7267523oig.152.1462462233080; Thu, 05 May 2016 08:30:33 -0700 (PDT) Received: by 10.202.81.76 with HTTP; Thu, 5 May 2016 08:30:32 -0700 (PDT) In-Reply-To: <1462446039-1070-5-git-send-email-michal.kazior@tieto.com> References: <1460636302-31161-1-git-send-email-michal.kazior@tieto.com> <1462446039-1070-1-git-send-email-michal.kazior@tieto.com> <1462446039-1070-5-git-send-email-michal.kazior@tieto.com> Date: Thu, 5 May 2016 08:30:32 -0700 Message-ID: From: Dave Taht To: Michal Kazior Cc: linux-wireless , Johannes Berg , make-wifi-fast@lists.bufferbloat.net, "codel@lists.bufferbloat.net" , Avery Pennarun Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Make-wifi-fast] [PATCHv4 4/5] mac80211: implement codel on fair queuing flows X-BeenThere: make-wifi-fast@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 May 2016 15:30:33 -0000 On Thu, May 5, 2016 at 4:00 AM, Michal Kazior wro= te: > There is no other limit other than a global > packet count limit when using software queuing. > This means a single flow queue can grow insanely > long. This is particularly bad for TCP congestion > algorithms which requires a little more > sophisticated frame dropping scheme than a mere > headdrop on limit overflow. > > Hence apply (a slighly modified, to fit the knobs) > CoDel5 on flow queues. This improves TCP > convergence and stability when combined with > wireless driver which keeps its own tx queue/fifo > at a minimum fill level for given link conditions. > > Signed-off-by: Michal Kazior > --- > > Notes: > v4: > * removed internal codel.h and re-used in-kernel one > > include/net/mac80211.h | 14 +++++- > net/mac80211/ieee80211_i.h | 5 +++ > net/mac80211/tx.c | 109 +++++++++++++++++++++++++++++++++++++++= +++++- > 3 files changed, 126 insertions(+), 2 deletions(-) > > diff --git a/include/net/mac80211.h b/include/net/mac80211.h > index ffb90dfe0d70..cc534f1b0f8e 100644 > --- a/include/net/mac80211.h > +++ b/include/net/mac80211.h > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > #include > > /** > @@ -895,7 +896,18 @@ struct ieee80211_tx_info { > unsigned long jiffies; > }; > /* NB: vif can be NULL for injected frames */ > - struct ieee80211_vif *vif; > + union { > + /* NB: vif can be NULL for injected frame= s */ > + struct ieee80211_vif *vif; > + > + /* When packets are enqueued on txq it's = easy > + * to re-construct the vif pointer. There= 's no > + * more space in tx_info so it can be use= d to > + * store the necessary enqueue time for p= acket > + * sojourn time computation. > + */ > + codel_time_t enqueue_time; > + }; Can't the skb->timestamp be used instead? (or does that still stomp on tcp) (my longstanding dream of course has been to always timestamp coming off the rx ring, and to not have to do it on entrance to the codel enqueue routine here. It adds measuring total system processing time to the queue measurement, allows for offloaded timestamping, etc, but did involve changing all of linux to use it) > struct ieee80211_key_conf *hw_key; > u32 flags; > /* 4 bytes free */ > diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h > index 6f8375f1df88..54edfb6fc1d1 100644 > --- a/net/mac80211/ieee80211_i.h > +++ b/net/mac80211/ieee80211_i.h > @@ -812,10 +812,12 @@ enum txq_info_flags { > * @tin: contains packets split into multiple flows > * @def_flow: used as a fallback flow when a packet destined to @tin has= hes to > * a fq_flow which is already owned by a different tin > + * @def_cvars: codel vars for @def_flow > */ > struct txq_info { > struct fq_tin tin; > struct fq_flow def_flow; > + struct codel_vars def_cvars; > unsigned long flags; > > /* keep last! */ > @@ -1108,6 +1110,9 @@ struct ieee80211_local { > struct ieee80211_hw hw; > > struct fq fq; > + struct codel_vars *cvars; > + struct codel_params cparams; > + struct codel_stats cstats; > > const struct ieee80211_ops *ops; > > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > index 47936b939591..013b382f6888 100644 > --- a/net/mac80211/tx.c > +++ b/net/mac80211/tx.c > @@ -25,6 +25,8 @@ > #include > #include > #include > +#include > +#include > #include > #include > > @@ -1269,11 +1271,92 @@ static struct txq_info *ieee80211_get_txq(struct = ieee80211_local *local, > return NULL; > } > > +static void ieee80211_set_skb_enqueue_time(struct sk_buff *skb) > +{ > + IEEE80211_SKB_CB(skb)->control.enqueue_time =3D codel_get_time(); > +} > + > +static void ieee80211_set_skb_vif(struct sk_buff *skb, struct txq_info *= txqi) > +{ > + IEEE80211_SKB_CB(skb)->control.vif =3D txqi->txq.vif; > +} > + > +static u32 codel_skb_len_func(const struct sk_buff *skb) > +{ > + return skb->len; > +} > + > +static codel_time_t codel_skb_time_func(const struct sk_buff *skb) > +{ > + const struct ieee80211_tx_info *info; > + > + info =3D (const struct ieee80211_tx_info *)skb->cb; > + return info->control.enqueue_time; > +} > + > +static struct sk_buff *codel_dequeue_func(struct codel_vars *cvars, > + void *ctx) > +{ > + struct ieee80211_local *local; > + struct txq_info *txqi; > + struct fq *fq; > + struct fq_flow *flow; > + > + txqi =3D ctx; > + local =3D vif_to_sdata(txqi->txq.vif)->local; > + fq =3D &local->fq; > + > + if (cvars =3D=3D &txqi->def_cvars) > + flow =3D &txqi->def_flow; > + else > + flow =3D &fq->flows[cvars - local->cvars]; > + > + return fq_flow_dequeue(fq, flow); > +} > + > +static void codel_drop_func(struct sk_buff *skb, > + void *ctx) > +{ > + struct ieee80211_local *local; > + struct ieee80211_hw *hw; > + struct txq_info *txqi; > + > + txqi =3D ctx; > + local =3D vif_to_sdata(txqi->txq.vif)->local; > + hw =3D &local->hw; > + > + ieee80211_free_txskb(hw, skb); > +} > + > static struct sk_buff *fq_tin_dequeue_func(struct fq *fq, > struct fq_tin *tin, > struct fq_flow *flow) > { > - return fq_flow_dequeue(fq, flow); > + struct ieee80211_local *local; > + struct txq_info *txqi; > + struct codel_vars *cvars; > + struct codel_params *cparams; > + struct codel_stats *cstats; > + > + local =3D container_of(fq, struct ieee80211_local, fq); > + txqi =3D container_of(tin, struct txq_info, tin); > + cparams =3D &local->cparams; > + cstats =3D &local->cstats; > + > + if (flow =3D=3D &txqi->def_flow) > + cvars =3D &txqi->def_cvars; > + else > + cvars =3D &local->cvars[flow - fq->flows]; > + > + return codel_dequeue(txqi, > + &flow->backlog, > + cparams, > + cvars, > + cstats, > + codel_skb_len_func, > + codel_skb_time_func, > + codel_drop_func, > + codel_dequeue_func); > } > > static void fq_skb_free_func(struct fq *fq, > @@ -1305,6 +1388,7 @@ static void ieee80211_txq_enqueue(struct ieee80211_= local *local, > struct fq *fq =3D &local->fq; > struct fq_tin *tin =3D &txqi->tin; > > + ieee80211_set_skb_enqueue_time(skb); > fq_tin_enqueue(fq, tin, skb, > fq_skb_free_func, > fq_flow_get_default_func); > @@ -1316,6 +1400,7 @@ void ieee80211_txq_init(struct ieee80211_sub_if_dat= a *sdata, > { > fq_tin_init(&txqi->tin); > fq_flow_init(&txqi->def_flow); > + codel_vars_init(&txqi->def_cvars); > > txqi->txq.vif =3D &sdata->vif; > > @@ -1344,6 +1429,7 @@ int ieee80211_txq_setup_flows(struct ieee80211_loca= l *local) > { > struct fq *fq =3D &local->fq; > int ret; > + int i; > > if (!local->ops->wake_tx_queue) > return 0; > @@ -1352,6 +1438,22 @@ int ieee80211_txq_setup_flows(struct ieee80211_loc= al *local) > if (ret) > return ret; > > + codel_params_init(&local->cparams); > + codel_stats_init(&local->cstats); > + local->cparams.interval =3D MS2TIME(100); > + local->cparams.target =3D MS2TIME(20); > + local->cparams.ecn =3D true; > + > + local->cvars =3D kcalloc(fq->flows_cnt, sizeof(local->cvars[0]), > + GFP_KERNEL); > + if (!local->cvars) { > + fq_reset(fq, fq_skb_free_func); > + return -ENOMEM; > + } > + > + for (i =3D 0; i < fq->flows_cnt; i++) > + codel_vars_init(&local->cvars[i]); > + > return 0; > } > > @@ -1362,6 +1464,9 @@ void ieee80211_txq_teardown_flows(struct ieee80211_= local *local) > if (!local->ops->wake_tx_queue) > return; > > + kfree(local->cvars); > + local->cvars =3D NULL; > + > fq_reset(fq, fq_skb_free_func); > } > > @@ -1384,6 +1489,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee802= 11_hw *hw, > if (!skb) > goto out; > > + ieee80211_set_skb_vif(skb, txqi); > + > hdr =3D (struct ieee80211_hdr *)skb->data; > if (txq->sta && ieee80211_is_data_qos(hdr->frame_control)) { > struct sta_info *sta =3D container_of(txq->sta, struct st= a_info, > -- > 2.1.4 > --=20 Dave T=C3=A4ht Let's go make home routers and wifi faster! With better software! http://blog.cerowrt.org