From d686b42e6c570a90d8356ffe91e1261e71661c9a Mon Sep 17 00:00:00 2001 From: "Anand H. Krishnan" Date: Thu, 27 Apr 2017 12:12:45 +0530 Subject: [PATCH] Reset the reach_nh pointer to original value If the composite nexthop does not have any component nexthops, we set the reach_nh to discard. One side effect of this behavior is that multicast ARPs are dropped, since ARP processing happens in the composite nexthop handling. Hence, retain the composite nexthop functionality, even if there are no component nexthops. As part of this change, we also make sure that if a change in any nexthop fails, we do not disrupt the way it works. Change-Id: Ic9ea091e3f0cbe659389bc57bea2a1e214bbd771 Closes-Bug: #1686452 --- dp-core/vr_nexthop.c | 231 +++++++++++++++++++++++++++++++------------ 1 file changed, 168 insertions(+), 63 deletions(-) diff --git a/dp-core/vr_nexthop.c b/dp-core/vr_nexthop.c index 7114c83b1..79b7627d0 100644 --- a/dp-core/vr_nexthop.c +++ b/dp-core/vr_nexthop.c @@ -593,6 +593,7 @@ nh_composite_ecmp(struct vr_packet *pkt, struct vr_nexthop *nh, struct vr_forwarding_md *fmd) { int ret = 0; + unsigned short drop_reason; struct vr_nexthop *member_nh = NULL; struct vr_vrf_stats *stats; @@ -601,8 +602,10 @@ nh_composite_ecmp(struct vr_packet *pkt, struct vr_nexthop *nh, if (stats) stats->vrf_ecmp_composites++; - if (!fmd) + if (!fmd) { + drop_reason = VP_DROP_NO_FMD; goto drop; + } if (fmd->fmd_ecmp_nh_index >= 0) { if (fmd->fmd_ecmp_nh_index < nh->nh_component_cnt) @@ -621,7 +624,7 @@ nh_composite_ecmp(struct vr_packet *pkt, struct vr_nexthop *nh, return nh_output(pkt, member_nh, fmd); drop: - vr_pfree(pkt, VP_DROP_NO_FMD); + vr_pfree(pkt, drop_reason); return ret; } @@ -898,6 +901,11 @@ nh_composite_mcast_l2(struct vr_packet *pkt, struct vr_nexthop *nh, return 0; + if (!nh->nh_component_cnt) { + drop_reason = VP_DROP_DISCARD; + goto drop; + } + /* * The packet can come to this nexthp either from Fabric or from VM. * Incase of Fabric, the packet would contain the Vxlan header and @@ -1039,6 +1047,11 @@ nh_composite_encap(struct vr_packet *pkt, struct vr_nexthop *nh, if (stats) stats->vrf_encap_composites++; + if (!nh->nh_component_cnt) { + drop_reason = VP_DROP_DISCARD; + goto drop; + } + if (!fmd) { drop_reason = VP_DROP_NO_FMD; goto drop; @@ -1085,6 +1098,11 @@ nh_composite_tor(struct vr_packet *pkt, struct vr_nexthop *nh, if (stats) stats->vrf_evpn_composites++; + if (!nh->nh_component_cnt) { + drop_reason = VP_DROP_DISCARD; + goto drop; + } + if (!fmd) { drop_reason = VP_DROP_NO_FMD; goto drop; @@ -1143,6 +1161,11 @@ nh_composite_evpn(struct vr_packet *pkt, struct vr_nexthop *nh, if (stats) stats->vrf_evpn_composites++; + if (!nh->nh_component_cnt) { + drop_reason = VP_DROP_DISCARD; + goto drop; + } + if (!fmd) { drop_reason = VP_DROP_NO_FMD; goto drop; @@ -1219,6 +1242,11 @@ nh_composite_fabric(struct vr_packet *pkt, struct vr_nexthop *nh, if (stats) stats->vrf_fabric_composites++; + if (!nh->nh_component_cnt) { + drop_reason = VP_DROP_DISCARD; + goto drop; + } + if (!fmd) { drop_reason = VP_DROP_NO_FMD; goto drop; @@ -2072,22 +2100,31 @@ nh_l2_rcv_add(struct vr_nexthop *nh, vr_nexthop_req *req) static int nh_rcv_add(struct vr_nexthop *nh, vr_nexthop_req *req) { + int ret = 0; struct vr_interface *vif, *old_vif; + + old_vif = nh->nh_dev; + vif = vrouter_get_interface(nh->nh_rid, req->nhr_encap_oif_id); - if (!vif) - return -ENODEV; + if (!vif) { + ret = -ENODEV; + goto exit_add; + } + /* * We need to delete the reference to old_vif only after new vif is * added to NH */ - old_vif = nh->nh_dev; nh->nh_dev = vif; - - nh->nh_reach_nh = nh_l3_rcv; - if (old_vif) vrouter_put_interface(old_vif); - return 0; + +exit_add: + if (nh->nh_dev) { + nh->nh_reach_nh = nh_l3_rcv; + } + + return ret; } static int @@ -2179,8 +2216,24 @@ nh_composite_mcast_validate(struct vr_nexthop *nh, vr_nexthop_req *req) static int nh_composite_add(struct vr_nexthop *nh, vr_nexthop_req *req) { + int ret = 0; unsigned int i; struct vr_nexthop *tmp_nh; + struct vr_component_nh *component_nh; + + if (req->nhr_nh_list_size != req->nhr_label_list_size) { + ret = -EINVAL; + goto exit_add; + } + + if (req->nhr_nh_list_size) { + component_nh = vr_zalloc(req->nhr_nh_list_size * + sizeof(struct vr_component_nh), VR_NEXTHOP_COMPONENT_OBJECT); + if (!component_nh) { + ret = -ENOMEM; + goto exit_add; + } + } nh->nh_validate_src = NULL; /* Delete the old nexthops first */ @@ -2194,18 +2247,11 @@ nh_composite_add(struct vr_nexthop *nh, vr_nexthop_req *req) nh->nh_component_cnt = 0; } - if (req->nhr_nh_list_size != req->nhr_label_list_size) - return -EINVAL; - /* Nh list of size 0 is valid */ if (req->nhr_nh_list_size == 0) - return 0; + goto exit_add; - nh->nh_component_nh = vr_zalloc(req->nhr_nh_list_size * - sizeof(struct vr_component_nh), VR_NEXTHOP_COMPONENT_OBJECT); - if (!nh->nh_component_nh) { - return -ENOMEM; - } + nh->nh_component_nh = component_nh; for (i = 0; i < req->nhr_nh_list_size; i++) { nh->nh_component_nh[i].cnh = vrouter_get_nexthop(req->nhr_rid, req->nhr_nh_list[i]); @@ -2216,6 +2262,7 @@ nh_composite_add(struct vr_nexthop *nh, vr_nexthop_req *req) if (nh_composite_mcast_validate(nh, req)) goto error; +exit_add: /* This needs to be the last */ if (req->nhr_flags & NH_FLAG_COMPOSITE_L2) { nh->nh_reach_nh = nh_composite_mcast_l2; @@ -2233,7 +2280,7 @@ nh_composite_add(struct vr_nexthop *nh, vr_nexthop_req *req) nh->nh_reach_nh = nh_composite_tor; } - return 0; + return ret; error: if (nh->nh_component_nh) { @@ -2250,30 +2297,66 @@ nh_composite_add(struct vr_nexthop *nh, vr_nexthop_req *req) return -EINVAL; } +static inline void +nh_tunnel_set_reach_nh(struct vr_nexthop *nh) +{ + bool dev = false; + + if (nh->nh_dev) { + dev = true; + } + + if (nh->nh_flags & NH_FLAG_TUNNEL_GRE) { + if (dev) { + nh->nh_reach_nh = nh_gre_tunnel; + } + } else if (nh->nh_flags & NH_FLAG_TUNNEL_UDP) { + nh->nh_reach_nh = nh_udp_tunnel; + } else if (nh->nh_flags & NH_FLAG_TUNNEL_UDP_MPLS) { + if (dev) { + nh->nh_reach_nh = nh_mpls_udp_tunnel; + } + } else if (nh->nh_flags & NH_FLAG_TUNNEL_VXLAN) { + if (dev) { + nh->nh_reach_nh = nh_vxlan_tunnel; + } + } + + return; +} + static int nh_tunnel_add(struct vr_nexthop *nh, vr_nexthop_req *req) { - struct vr_interface *vif, *old_vif; + int ret = 0; + struct vr_interface *vif, *old_vif = NULL; if (req->nhr_family == AF_INET6) { - if (!req->nhr_tun_sip6 || !req->nhr_tun_dip6) - return -EINVAL; + if (!req->nhr_tun_sip6 || !req->nhr_tun_dip6) { + ret = -EINVAL; + goto exit_add; + } } else { - if (!req->nhr_tun_sip || !req->nhr_tun_dip) - return -EINVAL; + if (!req->nhr_tun_sip || !req->nhr_tun_dip) { + ret = -EINVAL; + goto exit_add; + } } old_vif = nh->nh_dev; vif = vrouter_get_interface(nh->nh_rid, req->nhr_encap_oif_id); + if (nh->nh_flags & NH_FLAG_TUNNEL_GRE) { - if (!vif) - return -ENODEV; + if (!vif) { + ret = -ENODEV; + goto exit_add; + } + nh->nh_gre_tun_sip = req->nhr_tun_sip; nh->nh_gre_tun_dip = req->nhr_tun_dip; nh->nh_gre_tun_encap_len = req->nhr_encap_size; nh->nh_validate_src = nh_gre_tunnel_validate_src; nh->nh_dev = vif; - nh->nh_reach_nh = nh_gre_tunnel; } else if (nh->nh_flags & NH_FLAG_TUNNEL_UDP) { if (req->nhr_family == AF_INET) { nh->nh_udp_tun_sip = req->nhr_tun_sip; @@ -2285,51 +2368,60 @@ nh_tunnel_add(struct vr_nexthop *nh, vr_nexthop_req *req) if (!nh->nh_udp_tun6_sip) { nh->nh_udp_tun6_sip = vr_malloc(VR_IP6_ADDRESS_LEN, VR_NETWORK_ADDRESS_OBJECT); - if (!nh->nh_udp_tun6_sip) - return -ENOMEM; + if (!nh->nh_udp_tun6_sip) { + ret = -ENOMEM; + goto exit_error; + } } memcpy(nh->nh_udp_tun6_sip, req->nhr_tun_sip6, VR_IP6_ADDRESS_LEN); if (!nh->nh_udp_tun6_dip) { nh->nh_udp_tun6_dip = vr_malloc(VR_IP6_ADDRESS_LEN, VR_NETWORK_ADDRESS_OBJECT); - if (!nh->nh_udp_tun6_dip) - return -ENOMEM; + if (!nh->nh_udp_tun6_dip) { + ret = -ENOMEM; + goto exit_error; + } } - memcpy(nh->nh_udp_tun6_dip, req->nhr_tun_dip6, VR_IP6_ADDRESS_LEN); - + memcpy(nh->nh_udp_tun6_dip, req->nhr_tun_dip6, + VR_IP6_ADDRESS_LEN); nh->nh_udp_tun6_sport = req->nhr_tun_sport; nh->nh_udp_tun6_dport = req->nhr_tun_dport; nh->nh_udp_tun6_encap_len = req->nhr_encap_size; } else { - return -EINVAL; + ret = -EINVAL; + goto exit_error; } - nh->nh_reach_nh = nh_udp_tunnel; /* VIF should be null, but lets clean if one is found */ if (vif) vrouter_put_interface(vif); } else if (nh->nh_flags & NH_FLAG_TUNNEL_UDP_MPLS) { - if (!vif) - return -ENODEV; + if (!vif) { + ret = -ENODEV; + goto exit_add; + } + nh->nh_udp_tun_sip = req->nhr_tun_sip; nh->nh_udp_tun_dip = req->nhr_tun_dip; nh->nh_udp_tun_encap_len = req->nhr_encap_size; nh->nh_validate_src = nh_mpls_udp_tunnel_validate_src; nh->nh_dev = vif; - nh->nh_reach_nh = nh_mpls_udp_tunnel; } else if (nh->nh_flags & NH_FLAG_TUNNEL_VXLAN) { - if (!vif) - return -ENODEV; + if (!vif) { + ret = -ENODEV; + goto exit_add; + } + nh->nh_udp_tun_sip = req->nhr_tun_sip; nh->nh_udp_tun_dip = req->nhr_tun_dip; nh->nh_udp_tun_encap_len = req->nhr_encap_size; nh->nh_dev = vif; - nh->nh_reach_nh = nh_vxlan_tunnel; } else { /* Reference to VIf should be cleaned */ if (vif) vrouter_put_interface(vif); + return -EINVAL; } @@ -2337,45 +2429,54 @@ nh_tunnel_add(struct vr_nexthop *nh, vr_nexthop_req *req) if (old_vif) vrouter_put_interface(old_vif); - return 0; +exit_add: + nh_tunnel_set_reach_nh(nh); + +exit_error: + return ret; } static int nh_encap_add(struct vr_nexthop *nh, vr_nexthop_req *req) { + int ret = 0; struct vr_interface *vif, *old_vif;; - vif = vrouter_get_interface(nh->nh_rid, req->nhr_encap_oif_id); - if (!vif) - return -ENODEV; - - /* - * We need to delete the reference to old_vif only after new vif is - * added to NH - */ old_vif = nh->nh_dev; if (req->nhr_flags & NH_FLAG_ENCAP_L2) { if (req->nhr_encap_size < VR_ETHER_ALEN) { - vrouter_put_interface(vif); - return -EINVAL; + ret = -EINVAL; + goto exit_add; } - nh->nh_reach_nh = nh_encap_l2; - } else { - nh->nh_reach_nh = nh_encap_l3; - nh->nh_validate_src = nh_encap_l3_validate_src; } - nh->nh_dev = vif; + vif = vrouter_get_interface(nh->nh_rid, req->nhr_encap_oif_id); + if (!vif) { + ret = -EINVAL; + goto exit_add; + } + nh->nh_encap_family = req->nhr_encap_family; nh->nh_encap_len = req->nhr_encap_size; - if (nh->nh_encap_len && nh->nh_data) + if (nh->nh_encap_len && nh->nh_data) { memcpy(nh->nh_data, req->nhr_encap, nh->nh_encap_len); + } - + nh->nh_dev = vif; if (old_vif) vrouter_put_interface(old_vif); - return 0; +exit_add: + if (nh->nh_dev) { + if (req->nhr_flags & NH_FLAG_ENCAP_L2) { + nh->nh_reach_nh = nh_encap_l2; + } else { + nh->nh_validate_src = nh_encap_l3_validate_src; + nh->nh_reach_nh = nh_encap_l3; + } + } + + return ret; } static int @@ -2438,9 +2539,9 @@ int vr_nexthop_add(vr_nexthop_req *req) { int ret = 0, len = 0; + bool invalid_to_valid = false, change = false; struct vr_nexthop *nh; struct vrouter *router = vrouter_get(req->nhr_rid); - bool invalid_to_valid = false; if (!vr_nexthop_valid_request(req) && (ret = -EINVAL)) goto generate_resp; @@ -2460,6 +2561,7 @@ vr_nexthop_add(vr_nexthop_req *req) nh->nh_data_size = len - sizeof(struct vr_nexthop); } else { + change = true; /* * If modification of old_nh change the action to discard and ensure * everybody sees that @@ -2543,8 +2645,11 @@ vr_nexthop_add(vr_nexthop_req *req) } if (ret) { - if (nh->nh_destructor) - nh->nh_destructor(nh); + if (!change) { + if (nh->nh_destructor) { + nh->nh_destructor(nh); + } + } goto generate_resp; }