2
0
mirror of https://github.com/openvswitch/ovs synced 2025-08-29 13:27:59 +00:00

ofproto-dpif-upcall: Print more data on unassociated datapath ports.

When OVS fails to find an OpenFlow port for a packet received
from the upcall it just prints the warning like this:

  |INFO|received packet on unassociated datapath port N

However, during the flow translation more information is available
as if the recirculation id wasn't found or it was a packet from
unknown tunnel port.  Printing that information might be useful
to understand the origin of the problem.

Port translation functions already support extended error strings,
we just need to pass a variable where to store them.

With the change the output may be:

  |INFO|received packet on unassociated datapath port N
        (no OpenFlow port for datapath port N)
or
  |INFO|received packet on unassociated datapath port N
        (no OpenFlow tunnel port for this packet)
or
  |INFO|received packet on unassociated datapath port N
        (no recirculation data for recirc_id M)

Unfortunately, there is no good way to trigger this code from
current unit tests.

Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This commit is contained in:
Ilya Maximets 2022-09-01 17:42:49 +02:00
parent 0b21e23431
commit 96b26dce1d
3 changed files with 24 additions and 11 deletions

View File

@ -402,7 +402,8 @@ static int upcall_receive(struct upcall *, const struct dpif_backer *,
const struct dp_packet *packet, enum dpif_upcall_type, const struct dp_packet *packet, enum dpif_upcall_type,
const struct nlattr *userdata, const struct flow *, const struct nlattr *userdata, const struct flow *,
const unsigned int mru, const unsigned int mru,
const ovs_u128 *ufid, const unsigned pmd_id); const ovs_u128 *ufid, const unsigned pmd_id,
char **errorp);
static void upcall_uninit(struct upcall *); static void upcall_uninit(struct upcall *);
static void udpif_flow_rebalance(struct udpif *udpif); static void udpif_flow_rebalance(struct udpif *udpif);
@ -827,6 +828,7 @@ recv_upcalls(struct handler *handler)
struct upcall *upcall = &upcalls[n_upcalls]; struct upcall *upcall = &upcalls[n_upcalls];
struct flow *flow = &flows[n_upcalls]; struct flow *flow = &flows[n_upcalls];
unsigned int mru = 0; unsigned int mru = 0;
char *errorp = NULL;
uint64_t hash = 0; uint64_t hash = 0;
int error; int error;
@ -853,7 +855,7 @@ recv_upcalls(struct handler *handler)
error = upcall_receive(upcall, udpif->backer, &dupcall->packet, error = upcall_receive(upcall, udpif->backer, &dupcall->packet,
dupcall->type, dupcall->userdata, flow, mru, dupcall->type, dupcall->userdata, flow, mru,
&dupcall->ufid, PMD_ID_NULL); &dupcall->ufid, PMD_ID_NULL, &errorp);
if (error) { if (error) {
if (error == ENODEV) { if (error == ENODEV) {
/* Received packet on datapath port for which we couldn't /* Received packet on datapath port for which we couldn't
@ -864,8 +866,11 @@ recv_upcalls(struct handler *handler)
dupcall->key_len, NULL, 0, NULL, 0, dupcall->key_len, NULL, 0, NULL, 0,
&dupcall->ufid, PMD_ID_NULL, NULL); &dupcall->ufid, PMD_ID_NULL, NULL);
VLOG_INFO_RL(&rl, "received packet on unassociated datapath " VLOG_INFO_RL(&rl, "received packet on unassociated datapath "
"port %"PRIu32, flow->in_port.odp_port); "port %"PRIu32"%s%s%s", flow->in_port.odp_port,
errorp ? " (" : "", errorp ? errorp : "",
errorp ? ")" : "");
} }
free(errorp);
goto free_dupcall; goto free_dupcall;
} }
@ -1151,7 +1156,8 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer,
const struct dp_packet *packet, enum dpif_upcall_type type, const struct dp_packet *packet, enum dpif_upcall_type type,
const struct nlattr *userdata, const struct flow *flow, const struct nlattr *userdata, const struct flow *flow,
const unsigned int mru, const unsigned int mru,
const ovs_u128 *ufid, const unsigned pmd_id) const ovs_u128 *ufid, const unsigned pmd_id,
char **errorp)
{ {
int error; int error;
@ -1160,7 +1166,8 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer,
return EAGAIN; return EAGAIN;
} else if (upcall->type == MISS_UPCALL) { } else if (upcall->type == MISS_UPCALL) {
error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix,
&upcall->sflow, NULL, &upcall->ofp_in_port); &upcall->sflow, NULL, &upcall->ofp_in_port,
errorp);
if (error) { if (error) {
return error; return error;
} }
@ -1168,7 +1175,11 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer,
struct ofproto_dpif *ofproto struct ofproto_dpif *ofproto
= ofproto_dpif_lookup_by_uuid(&upcall->cookie.ofproto_uuid); = ofproto_dpif_lookup_by_uuid(&upcall->cookie.ofproto_uuid);
if (!ofproto) { if (!ofproto) {
VLOG_INFO_RL(&rl, "upcall could not find ofproto"); if (errorp) {
*errorp = xstrdup("upcall could not find ofproto");
} else {
VLOG_INFO_RL(&rl, "upcall could not find ofproto");
}
return ENODEV; return ENODEV;
} }
upcall->ofproto = ofproto; upcall->ofproto = ofproto;
@ -1358,7 +1369,7 @@ upcall_cb(const struct dp_packet *packet, const struct flow *flow, ovs_u128 *ufi
atomic_read_relaxed(&enable_megaflows, &megaflow); atomic_read_relaxed(&enable_megaflows, &megaflow);
error = upcall_receive(&upcall, udpif->backer, packet, type, userdata, error = upcall_receive(&upcall, udpif->backer, packet, type, userdata,
flow, 0, ufid, pmd_id); flow, 0, ufid, pmd_id, NULL);
if (error) { if (error) {
return error; return error;
} }
@ -2154,7 +2165,7 @@ xlate_key(struct udpif *udpif, const struct nlattr *key, unsigned int len,
} }
error = xlate_lookup(udpif->backer, &ctx->flow, &ofproto, NULL, NULL, error = xlate_lookup(udpif->backer, &ctx->flow, &ofproto, NULL, NULL,
ctx->netflow, &ofp_in_port); ctx->netflow, &ofp_in_port, NULL);
if (error) { if (error) {
return error; return error;
} }

View File

@ -1603,17 +1603,19 @@ xlate_lookup_ofproto(const struct dpif_backer *backer, const struct flow *flow,
* be taken. * be taken.
* *
* Returns 0 if successful, ENODEV if the parsed flow has no associated ofproto. * Returns 0 if successful, ENODEV if the parsed flow has no associated ofproto.
* Sets an extended error string to 'errorp'. Callers are responsible for
* freeing that string.
*/ */
int int
xlate_lookup(const struct dpif_backer *backer, const struct flow *flow, xlate_lookup(const struct dpif_backer *backer, const struct flow *flow,
struct ofproto_dpif **ofprotop, struct dpif_ipfix **ipfix, struct ofproto_dpif **ofprotop, struct dpif_ipfix **ipfix,
struct dpif_sflow **sflow, struct netflow **netflow, struct dpif_sflow **sflow, struct netflow **netflow,
ofp_port_t *ofp_in_port) ofp_port_t *ofp_in_port, char **errorp)
{ {
struct ofproto_dpif *ofproto; struct ofproto_dpif *ofproto;
const struct xport *xport; const struct xport *xport;
ofproto = xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport, NULL); ofproto = xlate_lookup_ofproto_(backer, flow, ofp_in_port, &xport, errorp);
if (!ofproto) { if (!ofproto) {
return ENODEV; return ENODEV;

View File

@ -209,7 +209,7 @@ struct ofproto_dpif * xlate_lookup_ofproto(const struct dpif_backer *,
int xlate_lookup(const struct dpif_backer *, const struct flow *, int xlate_lookup(const struct dpif_backer *, const struct flow *,
struct ofproto_dpif **, struct dpif_ipfix **, struct ofproto_dpif **, struct dpif_ipfix **,
struct dpif_sflow **, struct netflow **, struct dpif_sflow **, struct netflow **,
ofp_port_t *ofp_in_port); ofp_port_t *ofp_in_port, char **errorp);
const char *xlate_strerror(enum xlate_error error); const char *xlate_strerror(enum xlate_error error);