From 25825c6e373824e9abefb9a7fa3e9a4306f3d24d Mon Sep 17 00:00:00 2001 From: "Anand H. Krishnan" Date: Mon, 26 Oct 2015 11:25:12 +0530 Subject: [PATCH] Logic to identify the nature of label in fmd Post L2 flow support, vRouter started using the label in the forwarding metadata to also store VXLAN identifier. For MPLS-O-X(GRE/UDP) packets, the label indicates the MPLS label of the packet. For VXLAN tunneled packets, the label indicates the VNID. The logic that identifies this information has turned out to be buggy, resulting in VNID being identified as a MPLS label and thus sending the packet to a wrong interface/VM. Till now, we used to mark the label as VNID only if the packet had hit VRF translation nexthop (the vxlan kind) AND if the packet was cached in the flow entry because of that particular nexthop, which never happens and hence the label was always identified as a MPLS label. We now mark a flag indicating the type of the label, whenever the type is identified. To be more explicit, the label is now set in the fmd with an API that takes the type of the label, so that mistakes can be avoided. Closes BUG: 1509939 Change-Id: I63167986b4532d6820b0c76f267c61ca788e8fd4 --- dp-core/vr_bridge.c | 3 ++- dp-core/vr_flow.c | 18 +++++++++++------- dp-core/vr_mpls.c | 2 +- dp-core/vr_nexthop.c | 23 ++++++++++++++++------- dp-core/vr_proto_ip.c | 3 ++- dp-core/vr_vxlan.c | 3 ++- include/vr_flow.h | 2 +- include/vr_packet.h | 41 +++++++++++++++++++++++++++++++++++++++++ utils/rt.c | 8 ++++---- 9 files changed, 80 insertions(+), 23 deletions(-) diff --git a/dp-core/vr_bridge.c b/dp-core/vr_bridge.c index 023e4420f..0de3d3c41 100644 --- a/dp-core/vr_bridge.c +++ b/dp-core/vr_bridge.c @@ -546,7 +546,8 @@ vr_bridge_input(struct vrouter *router, struct vr_packet *pkt, vr_init_forwarding_md(&cmd); fmd = &cmd; } - fmd->fmd_label = rt.rtr_req.rtr_label; + vr_forwarding_md_set_label(fmd, rt.rtr_req.rtr_label, + VR_LABEL_TYPE_UNKNOWN); } if (pull_len && !pkt_push(pkt, pull_len)) { diff --git a/dp-core/vr_flow.c b/dp-core/vr_flow.c index 00ba130c1..ae55a974b 100644 --- a/dp-core/vr_flow.c +++ b/dp-core/vr_flow.c @@ -467,17 +467,14 @@ vr_flow_fill_pnode(struct vr_packet_node *pnode, struct vr_packet *pkt, * returns a different nexthop, in which case the ecmp index will return * a bad nexthop. to avoid that, we will cache the label, and reuse it */ - if (pkt->vp_nh && - (pkt->vp_nh->nh_type == NH_VRF_TRANSLATE) && - (pkt->vp_nh->nh_flags & NH_FLAG_VNID)) - pnode->pl_flags |= PN_FLAG_LABEL_IS_VNID; - pkt->vp_nh = NULL; pnode->pl_vif_idx = pkt->vp_if->vif_idx; if (fmd) { pnode->pl_outer_src_ip = fmd->fmd_outer_src_ip; pnode->pl_label = fmd->fmd_label; + if (vr_forwarding_md_label_is_vxlan_id(fmd)) + pnode->pl_flags |= PN_FLAG_LABEL_IS_VXLAN_ID; if (fmd->fmd_to_me) pnode->pl_flags |= PN_FLAG_TO_ME; } @@ -839,7 +836,14 @@ vr_flow_flush_pnode(struct vrouter *router, struct vr_packet_node *pnode, flow_result_t result; fmd->fmd_outer_src_ip = pnode->pl_outer_src_ip; - fmd->fmd_label = pnode->pl_label; + if (pnode->pl_flags & PN_FLAG_LABEL_IS_VXLAN_ID) { + vr_forwarding_md_set_label(fmd, pnode->pl_label, + VR_LABEL_TYPE_VXLAN_ID); + } else { + vr_forwarding_md_set_label(fmd, pnode->pl_label, + VR_LABEL_TYPE_MPLS); + } + if (pnode->pl_flags & PN_FLAG_TO_ME) fmd->fmd_to_me = 1; @@ -862,7 +866,7 @@ vr_flow_flush_pnode(struct vrouter *router, struct vr_packet_node *pnode, if (!pkt->vp_nh) { if (vif_is_fabric(pkt->vp_if) && fmd && (fmd->fmd_label >= 0)) { - if (!(pnode->pl_flags & PN_FLAG_LABEL_IS_VNID)) + if (!vr_forwarding_md_label_is_vxlan_id(fmd)) pkt->vp_nh = __vrouter_get_label(router, fmd->fmd_label); } } diff --git a/dp-core/vr_mpls.c b/dp-core/vr_mpls.c index 49c957735..a3a21b43b 100644 --- a/dp-core/vr_mpls.c +++ b/dp-core/vr_mpls.c @@ -324,7 +324,7 @@ vr_mpls_input(struct vrouter *router, struct vr_packet *pkt, ip = (struct vr_ip *)pkt_network_header(pkt); fmd->fmd_outer_src_ip = ip->ip_saddr; - fmd->fmd_label = label; + vr_forwarding_md_set_label(fmd, label, VR_LABEL_TYPE_MPLS); /* Store the TTL in packet. Will be used for multicast replication */ pkt->vp_ttl = ttl; diff --git a/dp-core/vr_nexthop.c b/dp-core/vr_nexthop.c index 04df32db5..ffd2f70f2 100644 --- a/dp-core/vr_nexthop.c +++ b/dp-core/vr_nexthop.c @@ -424,6 +424,8 @@ nh_vxlan_tunnel_helper(struct vr_packet *pkt, struct vr_forwarding_md *fmd, } } + vr_forwarding_md_update_label_type(fmd, VR_LABEL_TYPE_VXLAN_ID); + /* Add the vxlan header */ vxlanh = (struct vr_vxlan *)pkt_push(pkt, sizeof(struct vr_vxlan)); vxlanh->vxlan_vnid = htonl(fmd->fmd_label << VR_VXLAN_VNID_SHIFT); @@ -530,7 +532,9 @@ nh_composite_ecmp(struct vr_packet *pkt, struct vr_nexthop *nh, return 0; } - fmd->fmd_label = nh->nh_component_nh[fmd->fmd_ecmp_nh_index].cnh_label; + vr_forwarding_md_set_label(fmd, + nh->nh_component_nh[fmd->fmd_ecmp_nh_index].cnh_label, + VR_LABEL_TYPE_UNKNOWN); return nh_output(pkt, member_nh, fmd); drop: @@ -826,14 +830,13 @@ nh_composite_mcast_l2(struct vr_packet *pkt, struct vr_nexthop *nh, label = fmd->fmd_label; for (i = 0; i < nh->nh_component_cnt; i++) { - clone_size = 0; dir_nh = nh->nh_component_nh[i].cnh; /* We need to copy back the original label from Bridge lookaup * as previous iteration would have manipulated that */ - fmd->fmd_label = label; + vr_forwarding_md_set_label(fmd, label, VR_LABEL_TYPE_UNKNOWN); fmd->fmd_dvrf = pkt_vrf; /* If direct nexthop is not valid, dont process it */ @@ -1011,7 +1014,8 @@ nh_composite_tor(struct vr_packet *pkt, struct vr_nexthop *nh, break; } - fmd->fmd_label = nh->nh_component_nh[i].cnh_label; + vr_forwarding_md_set_label(fmd, nh->nh_component_nh[i].cnh_label, + VR_LABEL_TYPE_UNKNOWN); fmd->fmd_dvrf = dir_nh->nh_dev->vif_vrf; nh_output(new_pkt, dir_nh, fmd); } @@ -1063,7 +1067,8 @@ nh_composite_evpn(struct vr_packet *pkt, struct vr_nexthop *nh, break; } - fmd->fmd_label = nh->nh_component_nh[i].cnh_label; + vr_forwarding_md_set_label(fmd, nh->nh_component_nh[i].cnh_label, + VR_LABEL_TYPE_UNKNOWN); fmd->fmd_dvrf = dir_nh->nh_dev->vif_vrf; nh_output(new_pkt, dir_nh, fmd); } @@ -1184,7 +1189,7 @@ nh_composite_fabric(struct vr_packet *pkt, struct vr_nexthop *nh, * Add vxlan encapsulation. The vxlan id need to be taken * from Bridge entry */ - fmd->fmd_label = label; + vr_forwarding_md_set_label(fmd, label, VR_LABEL_TYPE_UNKNOWN); fmd->fmd_dvrf = dir_nh->nh_dev->vif_vrf; if (nh_vxlan_tunnel_helper(new_pkt, fmd, sip, sip) == false) { vr_pfree(new_pkt, VP_DROP_PUSH); @@ -1198,7 +1203,8 @@ nh_composite_fabric(struct vr_packet *pkt, struct vr_nexthop *nh, } /* MPLS label for outer header encapsulation */ - fmd->fmd_label = nh->nh_component_nh[i].cnh_label; + vr_forwarding_md_set_label(fmd, nh->nh_component_nh[i].cnh_label, + VR_LABEL_TYPE_UNKNOWN); fmd->fmd_dvrf = dir_nh->nh_dev->vif_vrf; nh_output(new_pkt, dir_nh, fmd); } @@ -1416,6 +1422,8 @@ nh_mpls_udp_tunnel(struct vr_packet *pkt, struct vr_nexthop *nh, if (!fmd || fmd->fmd_label < 0) return vr_forward(nh->nh_router, pkt, fmd); + vr_forwarding_md_update_label_type(fmd, VR_LABEL_TYPE_MPLS); + if (fmd->fmd_udp_src_port) udp_src_port = fmd->fmd_udp_src_port; @@ -1552,6 +1560,7 @@ nh_gre_tunnel(struct vr_packet *pkt, struct vr_nexthop *nh, if (!fmd || fmd->fmd_label < 0) return vr_forward(nh->nh_router, pkt, fmd); + vr_forwarding_md_update_label_type(fmd, VR_LABEL_TYPE_MPLS); if (vr_perfs) pkt->vp_flags |= VP_FLAG_GSO; diff --git a/dp-core/vr_proto_ip.c b/dp-core/vr_proto_ip.c index 0e32dcfc5..82e669fb7 100644 --- a/dp-core/vr_proto_ip.c +++ b/dp-core/vr_proto_ip.c @@ -253,7 +253,8 @@ vr_forward(struct vrouter *router, struct vr_packet *pkt, vr_init_forwarding_md(&rt_fmd); fmd = &rt_fmd; } - fmd->fmd_label = rt.rtr_req.rtr_label; + vr_forwarding_md_set_label(fmd, rt.rtr_req.rtr_label, + VR_LABEL_TYPE_UNKNOWN); } vif = nh->nh_dev; diff --git a/dp-core/vr_vxlan.c b/dp-core/vr_vxlan.c index 65d6a4d9d..95f98c075 100644 --- a/dp-core/vr_vxlan.c +++ b/dp-core/vr_vxlan.c @@ -42,7 +42,8 @@ vr_vxlan_input(struct vrouter *router, struct vr_packet *pkt, drop_reason = VP_DROP_PULL; goto fail; } - fmd->fmd_label = vnid; + + vr_forwarding_md_set_label(fmd, vnid, VR_LABEL_TYPE_VXLAN_ID); nh = (struct vr_nexthop *)vr_itable_get(router->vr_vxlan_table, vnid); if (!nh) { diff --git a/include/vr_flow.h b/include/vr_flow.h index 0db93e870..475a49682 100644 --- a/include/vr_flow.h +++ b/include/vr_flow.h @@ -143,7 +143,7 @@ struct vr_flow_stats { #define VR_MAX_FLOW_QUEUE_ENTRIES 3U -#define PN_FLAG_LABEL_IS_VNID 0x1 +#define PN_FLAG_LABEL_IS_VXLAN_ID 0x1 #define PN_FLAG_TO_ME 0x2 struct vr_packet_node { diff --git a/include/vr_packet.h b/include/vr_packet.h index cb13b8a7a..3070c0cd1 100644 --- a/include/vr_packet.h +++ b/include/vr_packet.h @@ -699,6 +699,12 @@ enum { TOR_SOURCE, }; +enum { + VR_LABEL_TYPE_UNKNOWN, + VR_LABEL_TYPE_MPLS, + VR_LABEL_TYPE_VXLAN_ID +}; + /* * forwarding metadata is something that is carried through out the * forwarding path. we are constrained by what can be held in the @@ -707,6 +713,8 @@ enum { * degradation, if so used. please also watch what you are doing with * this variable */ +#define FMD_FLAG_LABEL_IS_VXLAN_ID 0x01 + struct vr_forwarding_md { int32_t fmd_flow_index; int32_t fmd_label; @@ -718,6 +726,7 @@ struct vr_forwarding_md { uint16_t fmd_udp_src_port; uint8_t fmd_to_me; uint8_t fmd_src; + uint8_t fmd_flags; }; static inline void @@ -733,6 +742,38 @@ vr_init_forwarding_md(struct vr_forwarding_md *fmd) fmd->fmd_udp_src_port = 0; fmd->fmd_to_me = 0; fmd->fmd_src = 0; + fmd->fmd_flags = 0; + return; +} + +static inline bool +vr_forwarding_md_label_is_vxlan_id(struct vr_forwarding_md *fmd) +{ + if (fmd->fmd_flags & FMD_FLAG_LABEL_IS_VXLAN_ID) + return true; + return false; +} + +static inline void +vr_forwarding_md_update_label_type(struct vr_forwarding_md *fmd, + unsigned int type) +{ + if (type == VR_LABEL_TYPE_VXLAN_ID) { + fmd->fmd_flags |= FMD_FLAG_LABEL_IS_VXLAN_ID; + } else { + fmd->fmd_flags &= ~FMD_FLAG_LABEL_IS_VXLAN_ID; + } + + return; +} + +static inline void +vr_forwarding_md_set_label(struct vr_forwarding_md *fmd, unsigned int label, + unsigned int type) +{ + fmd->fmd_label = label; + vr_forwarding_md_update_label_type(fmd, type); + return; } diff --git a/utils/rt.c b/utils/rt.c index 25f0967e5..b29992de6 100644 --- a/utils/rt.c +++ b/utils/rt.c @@ -223,8 +223,8 @@ vr_route_req_process(void *s_req) if (rt->rtr_label_flags & VR_BE_FLOOD_DHCP_FLAG) strcat(flags, "Df"); - printf("%5d", rt->rtr_index); - for (i = 0; i < 5; i++) + printf("%-9d", rt->rtr_index); + for (i = 0; i < 3; i++) printf(" "); ret = printf("%s", ether_ntoa((struct ether_addr *)(rt->rtr_mac))); @@ -238,7 +238,7 @@ vr_route_req_process(void *s_req) else ret = printf(" %16c", '-'); - printf(" %10d\n", rt->rtr_nh_id); + printf(" %10d\n", rt->rtr_nh_id); } if (cmd_op != SANDESH_OP_DUMP) @@ -466,7 +466,7 @@ vr_route_op(void) } else { printf("Kernel L2 Bridge table %d/%d\n\n", req->rtr_rid, cmd_vrf_id); dump_legend(cmd_family_id); - printf("Index DestMac Flags Label/VNID Nexthop\n"); + printf("Index DestMac Flags Label/VNID Nexthop\n"); } }