| Message ID | 20200526150114.41687-4-dsahern@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | nexthops: Fix 2 fundamental flaws with nexthop groups | expand |
On 26/05/2020 18:01, David Ahern wrote: > From: David Ahern <dsahern@gmail.com> > > I got too fancy consolidating checks on multipath type. The result > is that path lookups can access 2 different nh_grp structs as exposed > by Nik's torture tests. Expand nexthop_is_multipath within nexthop.h to > avoid multiple, nh_grp dereferences and make decisions based on the > consistent struct. > > Only 2 places left using nexthop_is_multipath are within IPv6, both > only check that the nexthop is a multipath for a branching decision > which are acceptable. > > Fixes: 430a049190de ("nexthop: Add support for nexthop groups") > Signed-off-by: David Ahern <dsahern@gmail.com> > --- > include/net/nexthop.h | 41 +++++++++++++++++++++++++---------------- > 1 file changed, 25 insertions(+), 16 deletions(-) > Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> > diff --git a/include/net/nexthop.h b/include/net/nexthop.h > index 8a343519ed7a..f09e8d7d9886 100644 > --- a/include/net/nexthop.h > +++ b/include/net/nexthop.h > @@ -137,21 +137,20 @@ static inline unsigned int nexthop_num_path(const struct nexthop *nh) > { > unsigned int rc = 1; > > - if (nexthop_is_multipath(nh)) { > + if (nh->is_group) { > struct nh_group *nh_grp; > > nh_grp = rcu_dereference_rtnl(nh->nh_grp); > - rc = nh_grp->num_nh; > + if (nh_grp->mpath) > + rc = nh_grp->num_nh; > } > > return rc; > } > > static inline > -struct nexthop *nexthop_mpath_select(const struct nexthop *nh, int nhsel) > +struct nexthop *nexthop_mpath_select(const struct nh_group *nhg, int nhsel) > { > - const struct nh_group *nhg = rcu_dereference_rtnl(nh->nh_grp); > - > /* for_nexthops macros in fib_semantics.c grabs a pointer to > * the nexthop before checking nhsel > */ > @@ -186,12 +185,14 @@ static inline bool nexthop_is_blackhole(const struct nexthop *nh) > { > const struct nh_info *nhi; > > - if (nexthop_is_multipath(nh)) { > - if (nexthop_num_path(nh) > 1) > - return false; > - nh = nexthop_mpath_select(nh, 0); > - if (!nh) > + if (nh->is_group) { > + struct nh_group *nh_grp; > + > + nh_grp = rcu_dereference_rtnl(nh->nh_grp); > + if (nh_grp->num_nh > 1) > return false; > + > + nh = nh_grp->nh_entries[0].nh; > } > > nhi = rcu_dereference_rtnl(nh->nh_info); > @@ -217,10 +218,15 @@ struct fib_nh_common *nexthop_fib_nhc(struct nexthop *nh, int nhsel) > BUILD_BUG_ON(offsetof(struct fib_nh, nh_common) != 0); > BUILD_BUG_ON(offsetof(struct fib6_nh, nh_common) != 0); > > - if (nexthop_is_multipath(nh)) { > - nh = nexthop_mpath_select(nh, nhsel); > - if (!nh) > - return NULL; > + if (nh->is_group) { > + struct nh_group *nh_grp; > + > + nh_grp = rcu_dereference_rtnl(nh->nh_grp); > + if (nh_grp->mpath) { > + nh = nexthop_mpath_select(nh_grp, nhsel); > + if (!nh) > + return NULL; > + } > } > > nhi = rcu_dereference_rtnl(nh->nh_info); > @@ -264,8 +270,11 @@ static inline struct fib6_nh *nexthop_fib6_nh(struct nexthop *nh) > { > struct nh_info *nhi; > > - if (nexthop_is_multipath(nh)) { > - nh = nexthop_mpath_select(nh, 0); > + if (nh->is_group) { > + struct nh_group *nh_grp; > + > + nh_grp = rcu_dereference_rtnl(nh->nh_grp); > + nh = nexthop_mpath_select(nh_grp, 0); > if (!nh) > return NULL; > } >
diff --git a/include/net/nexthop.h b/include/net/nexthop.h index 8a343519ed7a..f09e8d7d9886 100644 --- a/include/net/nexthop.h +++ b/include/net/nexthop.h @@ -137,21 +137,20 @@ static inline unsigned int nexthop_num_path(const struct nexthop *nh) { unsigned int rc = 1; - if (nexthop_is_multipath(nh)) { + if (nh->is_group) { struct nh_group *nh_grp; nh_grp = rcu_dereference_rtnl(nh->nh_grp); - rc = nh_grp->num_nh; + if (nh_grp->mpath) + rc = nh_grp->num_nh; } return rc; } static inline -struct nexthop *nexthop_mpath_select(const struct nexthop *nh, int nhsel) +struct nexthop *nexthop_mpath_select(const struct nh_group *nhg, int nhsel) { - const struct nh_group *nhg = rcu_dereference_rtnl(nh->nh_grp); - /* for_nexthops macros in fib_semantics.c grabs a pointer to * the nexthop before checking nhsel */ @@ -186,12 +185,14 @@ static inline bool nexthop_is_blackhole(const struct nexthop *nh) { const struct nh_info *nhi; - if (nexthop_is_multipath(nh)) { - if (nexthop_num_path(nh) > 1) - return false; - nh = nexthop_mpath_select(nh, 0); - if (!nh) + if (nh->is_group) { + struct nh_group *nh_grp; + + nh_grp = rcu_dereference_rtnl(nh->nh_grp); + if (nh_grp->num_nh > 1) return false; + + nh = nh_grp->nh_entries[0].nh; } nhi = rcu_dereference_rtnl(nh->nh_info); @@ -217,10 +218,15 @@ struct fib_nh_common *nexthop_fib_nhc(struct nexthop *nh, int nhsel) BUILD_BUG_ON(offsetof(struct fib_nh, nh_common) != 0); BUILD_BUG_ON(offsetof(struct fib6_nh, nh_common) != 0); - if (nexthop_is_multipath(nh)) { - nh = nexthop_mpath_select(nh, nhsel); - if (!nh) - return NULL; + if (nh->is_group) { + struct nh_group *nh_grp; + + nh_grp = rcu_dereference_rtnl(nh->nh_grp); + if (nh_grp->mpath) { + nh = nexthop_mpath_select(nh_grp, nhsel); + if (!nh) + return NULL; + } } nhi = rcu_dereference_rtnl(nh->nh_info); @@ -264,8 +270,11 @@ static inline struct fib6_nh *nexthop_fib6_nh(struct nexthop *nh) { struct nh_info *nhi; - if (nexthop_is_multipath(nh)) { - nh = nexthop_mpath_select(nh, 0); + if (nh->is_group) { + struct nh_group *nh_grp; + + nh_grp = rcu_dereference_rtnl(nh->nh_grp); + nh = nexthop_mpath_select(nh_grp, 0); if (!nh) return NULL; }