On 26/05/2020 18:01, David Ahern wrote:
> From: David Ahern <dsah...@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 <dsah...@gmail.com>
> ---
> include/net/nexthop.h | 41 +++++++++++++++++++++++++----------------
> 1 file changed, 25 insertions(+), 16 deletions(-)
>
Acked-by: Nikolay Aleksandrov <niko...@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;
> }
>
