From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.toke.dk (mail.toke.dk [52.28.52.200]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id 771243CB39 for ; Wed, 16 May 2018 17:14:51 -0400 (EDT) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=toke.dk; s=20161023; t=1526505290; bh=+oXsHipNS1C5R4cEUA92bSRyMv3x3vYgKiNAOhMPduQ=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=wuoH9GWeRmElJDmZqPSWy3O1M/O0MSFeW9DOjv4ZypDqOF+A2Qx9B+1PyuGGxyWHt cCq6PtUDYFOl1+tJJv9muervJGqYzLiZv6EK1DNude1v2/qPXQ6X7pfOz+Fe8utXCd B18Mjl10btMLbHgCJQvMpztv2IEpYIZ0buA2ulJIyy3DIHX15VFqHxJVGr8QZS+sC2 u9VpYLcSSkqdMbZBvUsD3eiVIewyMkGUl3pBB83v7U/O7Z+0+/SUfVaXLsZBYejPZb DfQNkwlNsCeG5/ZFppMRJELEnm7/M/nTCIea8uLUiT2+b2+aR/MJ+xMRr/WO9jaKDQ FpcszaOF96m8A== To: Cong Wang Cc: Linux Kernel Network Developers , Cake List In-Reply-To: References: <152650253056.25701.10138252969621361651.stgit@alrua-kau> <152650254618.25701.1794377356779114652.stgit@alrua-kau> Date: Wed, 16 May 2018 23:14:44 +0200 X-Clacks-Overhead: GNU Terry Pratchett Message-ID: <87o9hfuwu3.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Cake] [PATCH net-next v12 4/7] sch_cake: Add NAT awareness to packet classifier X-BeenThere: cake@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Cake - FQ_codel the next generation List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 May 2018 21:14:51 -0000 Cong Wang writes: > On Wed, May 16, 2018 at 1:29 PM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> When CAKE is deployed on a gateway that also performs NAT (which is a >> common deployment mode), the host fairness mechanism cannot distinguish >> internal hosts from each other, and so fails to work correctly. >> >> To fix this, we add an optional NAT awareness mode, which will query the >> kernel conntrack mechanism to obtain the pre-NAT addresses for each pack= et >> and use that in the flow and host hashing. >> >> When the shaper is enabled and the host is already performing NAT, the c= ost >> of this lookup is negligible. However, in unlimited mode with no NAT bei= ng >> performed, there is a significant CPU cost at higher bandwidths. For this >> reason, the feature is turned off by default. >> >> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen >> --- >> net/sched/sch_cake.c | 73 +++++++++++++++++++++++++++++++++++++++++++= +++++++ >> 1 file changed, 73 insertions(+) >> >> diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c >> index 65439b643c92..e1038a7b6686 100644 >> --- a/net/sched/sch_cake.c >> +++ b/net/sched/sch_cake.c >> @@ -71,6 +71,12 @@ >> #include >> #include >> >> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK) >> +#include >> +#include >> +#include >> +#endif >> + >> #define CAKE_SET_WAYS (8) >> #define CAKE_MAX_TINS (8) >> #define CAKE_QUEUES (1024) >> @@ -514,6 +520,60 @@ static bool cobalt_should_drop(struct cobalt_vars *= vars, >> return drop; >> } >> >> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK) >> + >> +static void cake_update_flowkeys(struct flow_keys *keys, >> + const struct sk_buff *skb) >> +{ >> + const struct nf_conntrack_tuple *tuple; >> + enum ip_conntrack_info ctinfo; >> + struct nf_conn *ct; >> + bool rev =3D false; >> + >> + if (tc_skb_protocol(skb) !=3D htons(ETH_P_IP)) >> + return; >> + >> + ct =3D nf_ct_get(skb, &ctinfo); >> + if (ct) { >> + tuple =3D nf_ct_tuple(ct, CTINFO2DIR(ctinfo)); >> + } else { >> + const struct nf_conntrack_tuple_hash *hash; >> + struct nf_conntrack_tuple srctuple; >> + >> + if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), >> + NFPROTO_IPV4, dev_net(skb->dev), >> + &srctuple)) >> + return; >> + >> + hash =3D nf_conntrack_find_get(dev_net(skb->dev), >> + &nf_ct_zone_dflt, >> + &srctuple); >> + if (!hash) >> + return; >> + >> + rev =3D true; >> + ct =3D nf_ct_tuplehash_to_ctrack(hash); >> + tuple =3D nf_ct_tuple(ct, !hash->tuple.dst.dir); >> + } >> + >> + keys->addrs.v4addrs.src =3D rev ? tuple->dst.u3.ip : tuple->src.= u3.ip; >> + keys->addrs.v4addrs.dst =3D rev ? tuple->src.u3.ip : tuple->dst.= u3.ip; >> + >> + if (keys->ports.ports) { >> + keys->ports.src =3D rev ? tuple->dst.u.all : tuple->src.= u.all; >> + keys->ports.dst =3D rev ? tuple->src.u.all : tuple->dst.= u.all; >> + } >> + if (rev) >> + nf_ct_put(ct); >> +} >> +#else >> +static void cake_update_flowkeys(struct flow_keys *keys, >> + const struct sk_buff *skb) >> +{ >> + /* There is nothing we can do here without CONNTRACK */ >> +} >> +#endif >> + >> /* Cake has several subtle multiple bit settings. In these cases you >> * would be matching triple isolate mode as well. >> */ >> @@ -541,6 +601,9 @@ static u32 cake_hash(struct cake_tin_data *q, const = struct sk_buff *skb, >> skb_flow_dissect_flow_keys(skb, &keys, >> FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL); >> >> + if (flow_mode & CAKE_FLOW_NAT_FLAG) >> + cake_update_flowkeys(&keys, skb); >> + >> /* flow_hash_from_keys() sorts the addresses by value, so we have >> * to preserve their order in a separate data structure to treat >> * src and dst host addresses as independently selectable. >> @@ -1727,6 +1790,12 @@ static int cake_change(struct Qdisc *sch, struct = nlattr *opt, >> q->flow_mode =3D (nla_get_u32(tb[TCA_CAKE_FLOW_MODE]) & >> CAKE_FLOW_MASK); >> >> + if (tb[TCA_CAKE_NAT]) { >> + q->flow_mode &=3D ~CAKE_FLOW_NAT_FLAG; >> + q->flow_mode |=3D CAKE_FLOW_NAT_FLAG * >> + !!nla_get_u32(tb[TCA_CAKE_NAT]); >> + } > > > I think it's better to return -EOPNOTSUPP when CONFIG_NF_CONNTRACK > is not enabled. Good point, will fix :) -Toke