netdev - Re: [PATCH net] ipv4: include NLM_F_APPEND flag in append route notifications

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5581893E.3070609@cumulusnetworks.com>
Date:	Wed, 17 Jun 2015 07:50:38 -0700
From:	roopa <roopa@...ulusnetworks.com>
To:	Scott Feldman <sfeldma@...il.com>
CC:	"David S. Miller" <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] ipv4: include NLM_F_APPEND flag in append route notifications

On 6/17/15, 12:50 AM, Scott Feldman wrote:
> On Tue, Jun 16, 2015 at 9:11 AM, Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
>
> [snip]
>
>> @@ -1203,6 +1204,8 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg)
>>
>>                  if (!(cfg->fc_nlflags & NLM_F_APPEND))
>>                          fa = fa_first;
>> +               else
>> +                       nlflags |= NLM_F_APPEND;
>>          }
> The if and else parts above don't seem logically related.
I have it at this place because here is where the decision to append 
really happens.
>   Maybe you
> could initialize nlflags as:
>
>      unsigned int nlflags = cfg->fc_nlflags & (NLM_F_REPLACE|NLM_F_APPEND);
>
> And then pass rtmsg_fib(..., nlflags) to avoid the flag test/set?
nlflags should only contain NLM_F_REPLACE and NLM_F_APPEND if a replace or
append really took place. Hence the check and setting of nlflags is at 
the place where that
decision is made.

I had tried this patch a couple of other ways.... Do you think the below 
would be less confusing ?

thanks.



diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 3c699c4..9bfa3d8 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1082,6 +1082,7 @@ int fib_table_insert(struct fib_table *tb, struct 
fib_config *cfg)
         struct trie *t = (struct trie *)tb->tb_data;
         struct fib_alias *fa, *new_fa;
         struct key_vector *l, *tp;
+       unsigned int nlflags = 0;
         struct fib_info *fi;
         u8 plen = cfg->fc_dst_len;
         u8 slen = KEYLENGTH - plen;
@@ -1189,8 +1190,9 @@ int fib_table_insert(struct fib_table *tb, struct 
fib_config *cfg)
                         fib_release_info(fi_drop);
                         if (state & FA_S_ACCESSED)
rt_cache_flush(cfg->fc_nlinfo.nl_net);
+                       nlflags |= NLM_F_REPLACE;
                         rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen,
-                               tb->tb_id, &cfg->fc_nlinfo, NLM_F_REPLACE);
+                               tb->tb_id, &cfg->fc_nlinfo, nlflags);

                         goto succeeded;
                 }
@@ -1201,7 +1203,9 @@ int fib_table_insert(struct fib_table *tb, struct 
fib_config *cfg)
                 if (fa_match)
                         goto out;

-               if (!(cfg->fc_nlflags & NLM_F_APPEND))
+               if (cfg->fc_nlflags & NLM_F_APPEND)
+                       nlflags |= NLM_F_APPEND;
+               else
                         fa = fa_first;
         }
         err = -ENOENT;
@@ -1238,7 +1242,7 @@ int fib_table_insert(struct fib_table *tb, struct 
fib_config *cfg)

         rt_cache_flush(cfg->fc_nlinfo.nl_net);
         rtmsg_fib(RTM_NEWROUTE, htonl(key), new_fa, plen, new_fa->tb_id,
-                 &cfg->fc_nlinfo, 0);
+                 &cfg->fc_nlinfo, nlflags);
  succeeded:
         return 0;


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html