On Mon, Feb 26, 2018 at 1:10 AM, Paolo Abeni <pab...@redhat.com> wrote:
> On Sat, 2018-02-24 at 21:44 -0800, Roopa Prabhu wrote:
>> From: Roopa Prabhu <ro...@cumulusnetworks.com>
>>
>> Dissect flow in fwd path if fib rules require it. Controlled by
>> a flag to avoid penatly for the common case. Flag is set when fib
>> rules with sport, dport and proto match that require flow dissect
>> are installed. Also passes the dissected hash keys to the multipath
>> hash function when applicable to avoid dissecting the flow again.
>> icmp packets will continue to use inner header for hash
>> calculations (Thanks to Nikolay Aleksandrov for some review here).
>>
>> Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com>
>> ---
>> include/net/ip_fib.h | 2 +-
>> include/net/netns/ipv4.h | 1 +
>> net/ipv4/fib_rules.c | 6 ++++++
>> net/ipv4/fib_semantics.c | 2 +-
>> net/ipv4/route.c | 52
>> +++++++++++++++++++++++++++++++++++-------------
>> 5 files changed, 47 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
>> index f805243..5ada772 100644
>> --- a/include/net/ip_fib.h
>> +++ b/include/net/ip_fib.h
>> @@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int
>> nh_flags);
>>
>> #ifdef CONFIG_IP_ROUTE_MULTIPATH
>> int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
>> - const struct sk_buff *skb);
>> + const struct sk_buff *skb, struct flow_keys *flkeys);
>> #endif
>> void fib_select_multipath(struct fib_result *res, int hash);
>> void fib_select_path(struct net *net, struct fib_result *res,
>> diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
>> index 44668c2..87b8fdc 100644
>> --- a/include/net/netns/ipv4.h
>> +++ b/include/net/netns/ipv4.h
>> @@ -52,6 +52,7 @@ struct netns_ipv4 {
>> #ifdef CONFIG_IP_MULTIPLE_TABLES
>> struct fib_rules_ops *rules_ops;
>> bool fib_has_custom_rules;
>> + bool fib_rules_require_fldissect;
>> struct fib_table __rcu *fib_main;
>> struct fib_table __rcu *fib_default;
>> #endif
>> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
>> index 9d55c90..83aa786 100644
>> --- a/net/ipv4/fib_rules.c
>> +++ b/net/ipv4/fib_rules.c
>> @@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule,
>> struct sk_buff *skb,
>> }
>> #endif
>>
>> + if (rule->ip_proto ||
>> + fib_rule_port_range_valid(&rule->sport_range) ||
>> + fib_rule_port_range_valid(&rule->dport_range))
>> + net->ipv4.fib_rules_require_fldissect = true;
>> +
>> rule4->src_len = frh->src_len;
>> rule4->srcmask = inet_make_mask(rule4->src_len);
>> rule4->dst_len = frh->dst_len;
>
> What about using 'fib_rules_require_fldissect' to conditionally avoid
> all the tests introduced by patch 2/5 ? Perhaps even using a static key
> for that?
yep, ack
>
> It would be great if the kernel would be able to clear this flag when
> no more needed. I know it's not the current behaviour for other similar
> flags, but I hope we can improve ;)
>
yep agreed. I did lean on the existing fib rule flags behavior.
Nikolay suggested I can move this to a counter (which i had been
debating as well).
that gives the ability to clear on deletes.
> Both points above also apply to the ipv6 code path.
>
ack.
