Skip to content

Commit

Permalink
Logic to identify the nature of label in fmd
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@anandhk-juniper
anandhk-juniper committed Dec 29, 2015
1 parent fdeb3c5 commit 25825c6
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 23 deletions.
3 changes: 2 additions & 1 deletion dp-core/vr_bridge.c
View file
Expand Up @@ -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)) {
Expand Down
18 changes: 11 additions & 7 deletions dp-core/vr_flow.c
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;

Expand All @@ -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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion dp-core/vr_mpls.c
View file
Expand Up @@ -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;
Expand Down
23 changes: 16 additions & 7 deletions dp-core/vr_nexthop.c
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion dp-core/vr_proto_ip.c
View file
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion dp-core/vr_vxlan.c
View file
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion include/vr_flow.h
View file
Expand Up @@ -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 {
Expand Down
41 changes: 41 additions & 0 deletions include/vr_packet.h
View file
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
8 changes: 4 additions & 4 deletions utils/rt.c
View file
Expand Up @@ -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)));
Expand All @@ -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)
Expand Down Expand Up @@ -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");
}
}

Expand Down

0 comments on commit 25825c6

Please sign in to comment.