Merge pull request FRRouting#9926 from donaldsharp/update_issues

zebra: Fix v6 route replace failure turned into success

zebra/kernel_netlink.c

@@ -1125,8 +1125,25 @@ static int nl_batch_read_resp(struct nl_batch *bth)
  while (true) {
  status = netlink_recv_msg(nl, msg, nl_batch_rx_buf,
  sizeof(nl_batch_rx_buf));
- if (status == -1 || status == 0)
+ /*
+ * status == -1 is a full on failure somewhere
+ * since we don't know where the problem happened
+ * we must mark all as failed
+ *
+ * Else we mark everything as worked
+ *
+ */
+ if (status == -1 || status == 0) {
+ while ((ctx = dplane_ctx_dequeue(&(bth->ctx_list))) !=
+ NULL) {
+ if (status == -1)
+ dplane_ctx_set_status(
+ ctx,
+ ZEBRA_DPLANE_REQUEST_FAILURE);
+ dplane_ctx_enqueue_tail(bth->ctx_out_q, ctx);
+ }
  return status;
+ }
 
  h = (struct nlmsghdr *)nl_batch_rx_buf;
  ignore_msg = false;
@@ -1138,15 +1155,18 @@ static int nl_batch_read_resp(struct nl_batch *bth)
  * requests at same time.
  */
  while (true) {
- ctx = dplane_ctx_dequeue(&(bth->ctx_list));
- if (ctx == NULL)
- break;
-
- dplane_ctx_enqueue_tail(bth->ctx_out_q, ctx);
-
- /* We have found corresponding context object. */
- if (dplane_ctx_get_ns(ctx)->nls.seq == seq)
+ ctx = dplane_ctx_get_head(&(bth->ctx_list));
+ if (ctx == NULL) {
+ /*
+ * This is a situation where we have gotten
+ * into a bad spot. We need to know that
+ * this happens( does it? )
+ */
+ zlog_err(
+ "%s:WARNING Received netlink Response for an error and no Contexts to associate with it",
+ __func__);
  break;
+ }
 
  /*
  * 'update' context objects take two consecutive
@@ -1161,10 +1181,35 @@ static int nl_batch_read_resp(struct nl_batch *bth)
  ignore_msg = true;
  break;
  }
+
+ ctx = dplane_ctx_dequeue(&(bth->ctx_list));
+ dplane_ctx_enqueue_tail(bth->ctx_out_q, ctx);
+
+ /* We have found corresponding context object. */
+ if (dplane_ctx_get_ns(ctx)->nls.seq == seq)
+ break;
+
+ if (dplane_ctx_get_ns(ctx)->nls.seq > seq)
+ zlog_warn(
+ "%s:WARNING Recieved %u is less than any context on the queue ctx->seq %u",
+ __func__, seq,
+ dplane_ctx_get_ns(ctx)->nls.seq);
  }
 
- if (ignore_msg)
+ if (ignore_msg) {
+ /*
+ * If we ignore the message due to an update
+ * above we should still fricking decode the
+ * message for our operator to understand
+ * what is going on
+ */
+ int err = netlink_parse_error(nl, h, bth->zns->is_cmd,
+ false);
+
+ zlog_debug("%s: netlink error message seq=%d %d",
+ __func__, h->nlmsg_seq, err);
  continue;
+ }
 
  /*
  * We received a message with the sequence number that isn't

zebra/zebra_dplane.c

@@ -833,6 +833,13 @@ void dplane_ctx_list_append(struct dplane_ctx_q *to_list,
  }
 }
 
+struct zebra_dplane_ctx *dplane_ctx_get_head(struct dplane_ctx_q *q)
+{
+ struct zebra_dplane_ctx *ctx = TAILQ_FIRST(q);
+
+ return ctx;
+}
+
 /* Dequeue a context block from the head of a list */
 struct zebra_dplane_ctx *dplane_ctx_dequeue(struct dplane_ctx_q *q)
 {
@@ -2296,6 +2303,8 @@ static int dplane_ctx_ns_init(struct zebra_dplane_ctx *ctx,
 {
  dplane_info_from_zns(&(ctx->zd_ns_info), zns);
 
+ ctx->zd_is_update = is_update;
+
 #if defined(HAVE_NETLINK)
  /* Increment message counter after copying to context struct - may need
  * two messages in some 'update' cases.
@@ -2514,7 +2523,6 @@ int dplane_ctx_nexthop_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
  * it probably won't require two messages
  */
  dplane_ctx_ns_init(ctx, zns, (op == DPLANE_OP_NH_UPDATE));
- ctx->zd_is_update = (op == DPLANE_OP_NH_UPDATE);
 
  ret = AOK;
 
@@ -2537,7 +2545,6 @@ int dplane_ctx_lsp_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
  /* Capture namespace info */
  dplane_ctx_ns_init(ctx, zebra_ns_lookup(NS_DEFAULT),
  (op == DPLANE_OP_LSP_UPDATE));
- ctx->zd_is_update = (op == DPLANE_OP_LSP_UPDATE);
 
  memset(&ctx->u.lsp, 0, sizeof(ctx->u.lsp));
 
@@ -2813,7 +2820,6 @@ static int dplane_ctx_rule_init(struct zebra_dplane_ctx *ctx,
 
  dplane_ctx_ns_init(ctx, zebra_ns_lookup(NS_DEFAULT),
  op == DPLANE_OP_RULE_UPDATE);
- ctx->zd_is_update = (op == DPLANE_OP_RULE_UPDATE);
 
  ctx->zd_vrf_id = new_rule->vrf_id;
  strlcpy(ctx->zd_ifname, new_rule->ifname, sizeof(ctx->zd_ifname));
@@ -2859,7 +2865,6 @@ static int dplane_ctx_iptable_init(struct zebra_dplane_ctx *ctx,
  ctx->zd_status = ZEBRA_DPLANE_REQUEST_SUCCESS;
 
  dplane_ctx_ns_init(ctx, zebra_ns_lookup(NS_DEFAULT), false);
- ctx->zd_is_update = false;
 
  ctx->zd_vrf_id = iptable->vrf_id;
  memcpy(&ctx->u.iptable, iptable, sizeof(struct zebra_pbr_iptable));
@@ -2899,7 +2904,6 @@ static int dplane_ctx_ipset_init(struct zebra_dplane_ctx *ctx,
  ctx->zd_status = ZEBRA_DPLANE_REQUEST_SUCCESS;
 
  dplane_ctx_ns_init(ctx, zebra_ns_lookup(NS_DEFAULT), false);
- ctx->zd_is_update = false;
 
  ctx->zd_vrf_id = ipset->vrf_id;
 
@@ -2934,7 +2938,6 @@ dplane_ctx_ipset_entry_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
  ctx->zd_status = ZEBRA_DPLANE_REQUEST_SUCCESS;
 
  dplane_ctx_ns_init(ctx, zebra_ns_lookup(NS_DEFAULT), false);
- ctx->zd_is_update = false;
 
  ctx->zd_vrf_id = ipset->vrf_id;
 
@@ -3015,7 +3018,6 @@ dplane_route_update_internal(struct route_node *rn,
  */
  if ((op == DPLANE_OP_ROUTE_UPDATE) &&
  old_re && (old_re != re)) {
- ctx->zd_is_update = true;
 
  old_re->dplane_sequence =
  zebra_router_get_next_sequence();

zebra/zebra_dplane.h

@@ -274,6 +274,7 @@ void dplane_ctx_list_append(struct dplane_ctx_q *to_list,
 
 /* Dequeue a context block from the head of caller's tailq */
 struct zebra_dplane_ctx *dplane_ctx_dequeue(struct dplane_ctx_q *q);
+struct zebra_dplane_ctx *dplane_ctx_get_head(struct dplane_ctx_q *q);
 
 /*
  * Accessors for information from the context object