Skip to content

Commit

Permalink
ofproto: Remove double reporting from bundles.
Browse files Browse the repository at this point in the history
Patch b0d38b2 unified flow mod reporting in ofproto for both
stand-alone flow mods and bundle flow mods, but left bundle-specific
reporting to the bundle removal code.  This patch fixes this by
removing the bundle-specific reporting of flow mods.

Found by inspection.

Fixes: b0d38b2 ("ofproto: Report flow mods also from bundles.")
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
jrajahalme committed Sep 15, 2016
1 parent f069903 commit 0c78eeb
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 17 deletions.
20 changes: 7 additions & 13 deletions ofproto/bundles.c
Expand Up @@ -62,16 +62,11 @@ ofp_bundle_create(uint32_t id, uint16_t flags, const struct ofp_header *oh)
}

void
ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle *bundle,
bool success)
ofp_bundle_remove__(struct ofconn *ofconn, struct ofp_bundle *bundle)
{
struct ofp_bundle_entry *msg;

LIST_FOR_EACH_POP (msg, node, &bundle->msg_list) {
if (success && msg->type == OFPTYPE_FLOW_MOD) {
/* Tell connmgr about successful flow mods. */
ofconn_report_flow_mod(ofconn, msg->ofm.command);
}
ofp_bundle_entry_free(msg);
}

Expand All @@ -90,7 +85,7 @@ ofp_bundle_open(struct ofconn *ofconn, uint32_t id, uint16_t flags,

if (bundle) {
VLOG_INFO("Bundle %x already exists.", id);
ofp_bundle_remove__(ofconn, bundle, false);
ofp_bundle_remove__(ofconn, bundle);

return OFPERR_OFPBFC_BAD_ID;
}
Expand All @@ -116,12 +111,12 @@ ofp_bundle_close(struct ofconn *ofconn, uint32_t id, uint16_t flags)
}

if (bundle->state == BS_CLOSED) {
ofp_bundle_remove__(ofconn, bundle, false);
ofp_bundle_remove__(ofconn, bundle);
return OFPERR_OFPBFC_BUNDLE_CLOSED;
}

if (bundle->flags != flags) {
ofp_bundle_remove__(ofconn, bundle, false);
ofp_bundle_remove__(ofconn, bundle);
return OFPERR_OFPBFC_BAD_FLAGS;
}

Expand All @@ -141,8 +136,7 @@ ofp_bundle_discard(struct ofconn *ofconn, uint32_t id)
return OFPERR_OFPBFC_BAD_ID;
}

ofp_bundle_remove__(ofconn, bundle, false);

ofp_bundle_remove__(ofconn, bundle);
return 0;
}

Expand All @@ -165,10 +159,10 @@ ofp_bundle_add_message(struct ofconn *ofconn, uint32_t id, uint16_t flags,
return error;
}
} else if (bundle->state == BS_CLOSED) {
ofp_bundle_remove__(ofconn, bundle, false);
ofp_bundle_remove__(ofconn, bundle);
return OFPERR_OFPBFC_BUNDLE_CLOSED;
} else if (flags != bundle->flags) {
ofp_bundle_remove__(ofconn, bundle, false);
ofp_bundle_remove__(ofconn, bundle);
return OFPERR_OFPBFC_BAD_FLAGS;
}

Expand Down
2 changes: 1 addition & 1 deletion ofproto/bundles.h
Expand Up @@ -83,7 +83,7 @@ enum ofperr ofp_bundle_add_message(struct ofconn *, uint32_t id,
uint16_t flags, struct ofp_bundle_entry *,
const struct ofp_header *);

void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *, bool success);
void ofp_bundle_remove__(struct ofconn *, struct ofp_bundle *);

static inline struct ofp_bundle_entry *
ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh)
Expand Down
4 changes: 2 additions & 2 deletions ofproto/connmgr.c
Expand Up @@ -1234,7 +1234,7 @@ bundle_remove_all(struct ofconn *ofconn)
struct ofp_bundle *b, *next;

HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) {
ofp_bundle_remove__(ofconn, b, false);
ofp_bundle_remove__(ofconn, b);
}
}

Expand All @@ -1247,7 +1247,7 @@ bundle_remove_expired(struct ofconn *ofconn, long long int now)
HMAP_FOR_EACH_SAFE (b, next, node, &ofconn->bundles) {
if (b->used <= limit) {
ofconn_send_error(ofconn, &b->ofp_msg, OFPERR_OFPBFC_TIMEOUT);
ofp_bundle_remove__(ofconn, b, false);
ofp_bundle_remove__(ofconn, b);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion ofproto/ofproto.c
Expand Up @@ -7590,7 +7590,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t id, uint16_t flags)
}

/* The bundle is discarded regardless the outcome. */
ofp_bundle_remove__(ofconn, bundle, !error);
ofp_bundle_remove__(ofconn, bundle);
return error;
}

Expand Down
7 changes: 7 additions & 0 deletions tests/ofproto.at
Expand Up @@ -5008,6 +5008,13 @@ vconn|DBG|unix: sent (Success): NXST_FLOW reply:
in_port=2,dl_src=00:66:77:88:99:aa actions=drop
])

AT_CHECK([grep " flow_mods in the last " ovs-vswitchd.log | sed -e 's/^.*connmgr|INFO|//'], [0], [dnl
br0<->unix: 1 flow_mods in the last 0 s (1 deletes)
br0<->unix: 9 flow_mods in the last 0 s (7 adds, 2 deletes)
br0<->unix: 2 flow_mods in the last 0 s (2 modifications)
br0<->unix: 3 flow_mods in the last 0 s (2 adds, 1 deletes)
])

OVS_VSWITCHD_STOP
AT_CLEANUP

Expand Down

0 comments on commit 0c78eeb

Please sign in to comment.