Skip to content

Commit

Permalink
If a pclone fails, trap the original packet
Browse files Browse the repository at this point in the history
In the process of indicating a flow miss to the agent, if a pclone
fails, vRouter returns without unsetting the NEW flag, a flag that
indicates that the datapath is in the process of creating a new flow.
The side effect of not unsetting this flag is that agent can not
modify the entry and this leads to perennial HOLD flows in the
system.

To fix this problem, if the pclone fails, trap the original packet
to the agent without holding it in the HOLD queue. While this will
mean that the first packet does not reach the destination, it is
a better alternative to not trapping the packet and wait for agent
to do the audit after a minute or so. Such trapped packets are
captured under a new counter "Original Packet Trapped" in the
dropstats output.

Change-Id: I34c19abf935d9b06f55e875b76a4859350743c2b
Partial-Bug: #1628175
  • Loading branch information
anandhk-juniper committed Sep 30, 2016
1 parent 90c9693 commit 8d38dc3
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 15 deletions.
19 changes: 13 additions & 6 deletions dp-core/vr_flow.c
Expand Up @@ -64,7 +64,8 @@ static bool vr_flow_is_fat_flow(struct vrouter *, struct vr_packet *,
struct vr_flow_entry *vr_find_flow(struct vrouter *, struct vr_flow *,
uint8_t, unsigned int *);
unsigned int vr_trap_flow(struct vrouter *, struct vr_flow_entry *,
struct vr_packet *, unsigned int, struct vr_flow_stats *);
struct vr_packet *, unsigned int, struct vr_flow_stats *,
struct vr_packet_node *);

void get_random_bytes(void *buf, int nbytes);

Expand Down Expand Up @@ -602,6 +603,7 @@ vr_enqueue_flow(struct vrouter *router, struct vr_flow_entry *fe,
struct vr_packet *pkt, unsigned int index,
struct vr_flow_stats *stats, struct vr_forwarding_md *fmd)
{
int ret = 0;
unsigned int i;
unsigned short drop_reason = 0;
struct vr_flow_queue *vfq = fe->fe_hold_list;
Expand All @@ -621,9 +623,9 @@ vr_enqueue_flow(struct vrouter *router, struct vr_flow_entry *fe,
pnode = &vfq->vfq_pnodes[i];
vr_flow_fill_pnode(pnode, pkt, fmd);
if (!i)
vr_trap_flow(router, fe, pkt, index, stats);
ret = vr_trap_flow(router, fe, pkt, index, stats, pnode);

return 0;
return ret;
drop:
vr_pfree(pkt, drop_reason);
return 0;
Expand Down Expand Up @@ -837,15 +839,20 @@ vr_flow_action(struct vrouter *router, struct vr_flow_entry *fe,
unsigned int
vr_trap_flow(struct vrouter *router, struct vr_flow_entry *fe,
struct vr_packet *pkt, unsigned int index,
struct vr_flow_stats *stats)
struct vr_flow_stats *stats, struct vr_packet_node *pnode)
{
unsigned int trap_reason;

struct vr_packet *npkt;
struct vr_flow_trap_arg ta;

npkt = vr_pclone(pkt);
if (!npkt)
return -ENOMEM;
if (!npkt) {
vr_pfree(NULL, VP_DROP_TRAP_ORIGINAL);
if (pnode)
pnode->pl_packet = NULL;
npkt = pkt;
}

vr_preset(npkt);

Expand Down
1 change: 1 addition & 0 deletions dp-core/vr_stats.c
Expand Up @@ -67,6 +67,7 @@ vr_drop_stats_add_response(vr_drop_stats_req *response,
response->vds_fragment_queue_fail += stats->vds_fragment_queue_fail;
response->vds_vlan_fwd_tx += stats->vds_vlan_fwd_tx;
response->vds_vlan_fwd_enq += stats->vds_vlan_fwd_enq;
response->vds_trap_original += stats->vds_trap_original;

return;
}
Expand Down
5 changes: 4 additions & 1 deletion dpdk/vr_dpdk_host.c
Expand Up @@ -154,7 +154,10 @@ dpdk_pfree(struct vr_packet *pkt, unsigned short reason)

router->vr_pdrop_stats[rte_lcore_id()][reason]++;

rte_pktmbuf_free(vr_dpdk_pkt_to_mbuf(pkt));
if (pkt)
rte_pktmbuf_free(vr_dpdk_pkt_to_mbuf(pkt));

return;
}

void
Expand Down
5 changes: 3 additions & 2 deletions include/vr_packet.h
Expand Up @@ -179,7 +179,8 @@
#define VP_DROP_VLAN_FWD_TX 45
#define VP_DROP_VLAN_FWD_ENQ 46
#define VP_DROP_FLOW_EVICT 47
#define VP_DROP_MAX 48
#define VP_DROP_TRAP_ORIGINAL 48
#define VP_DROP_MAX 49


struct vr_drop_stats {
Expand Down Expand Up @@ -231,7 +232,7 @@ struct vr_drop_stats {
uint64_t vds_vlan_fwd_tx;
uint64_t vds_vlan_fwd_enq;
uint64_t vds_flow_evict;

uint64_t vds_trap_original;
};

/*
Expand Down
22 changes: 16 additions & 6 deletions linux/vrouter_mod.c
Expand Up @@ -78,6 +78,7 @@ extern void vhost_exit(void);
extern int lh_gro_process(struct vr_packet *, struct vr_interface *, bool);

static void lh_reset_skb_fields(struct vr_packet *pkt);
static unsigned int lh_get_cpu(void);

static int
lh_printk(const char *format, ...)
Expand Down Expand Up @@ -302,17 +303,26 @@ lh_pgso_size(struct vr_packet *pkt)
static void
lh_pfree(struct vr_packet *pkt, unsigned short reason)
{
unsigned int cpu;

struct vrouter *router = vrouter_get(0);
struct sk_buff *skb;
struct sk_buff *skb = NULL;

skb = vp_os_packet(pkt);
if (!skb)
return;
if (pkt) {
skb = vp_os_packet(pkt);
if (!skb)
return;
cpu = pkt->vp_cpu;
} else {
cpu = lh_get_cpu();
}

if (router)
((uint64_t *)(router->vr_pdrop_stats[pkt->vp_cpu]))[reason]++;
((uint64_t *)(router->vr_pdrop_stats[cpu]))[reason]++;

if (skb)
kfree_skb(skb);

kfree_skb(skb);
return;
}

Expand Down
1 change: 1 addition & 0 deletions sandesh/vr.sandesh
Expand Up @@ -388,4 +388,5 @@ buffer sandesh vr_drop_stats_req {
50: i64 vds_vlan_fwd_tx;
51: i64 vds_vlan_fwd_enq;
52: i64 vds_flow_evict;
53: i64 vds_trap_original;
}
4 changes: 4 additions & 0 deletions utils/dropstats.c
Expand Up @@ -79,6 +79,10 @@ vr_drop_stats_req_process(void *s_req)
stats->vds_flow_evict);
printf("\n");

printf("Original Packet Trapped %" PRIu64 "\n",
stats->vds_trap_original);
printf("\n");

printf("Discards %" PRIu64 "\n",
stats->vds_discard);
printf("TTL Exceeded %" PRIu64 "\n",
Expand Down

0 comments on commit 8d38dc3

Please sign in to comment.