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.

Signed-off-by: Ilya Maximets <[email protected]>
---
 ofproto/ofproto-dpif-upcall.c | 27 +++++++++++++++++++--------
 ofproto/ofproto-dpif-xlate.c  |  6 ++++--
 ofproto/ofproto-dpif-xlate.h  |  2 +-
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 57f94df54..58671a8aa 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -398,7 +398,8 @@ static int upcall_receive(struct upcall *, const struct 
dpif_backer *,
                           const struct dp_packet *packet, enum 
dpif_upcall_type,
                           const struct nlattr *userdata, const struct flow *,
                           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 udpif_flow_rebalance(struct udpif *udpif);
@@ -819,6 +820,7 @@ recv_upcalls(struct handler *handler)
         struct upcall *upcall = &upcalls[n_upcalls];
         struct flow *flow = &flows[n_upcalls];
         unsigned int mru = 0;
+        char *errorp = NULL;
         uint64_t hash = 0;
         int error;
 
@@ -845,7 +847,7 @@ recv_upcalls(struct handler *handler)
 
         error = upcall_receive(upcall, udpif->backer, &dupcall->packet,
                                dupcall->type, dupcall->userdata, flow, mru,
-                               &dupcall->ufid, PMD_ID_NULL);
+                               &dupcall->ufid, PMD_ID_NULL, &errorp);
         if (error) {
             if (error == ENODEV) {
                 /* Received packet on datapath port for which we couldn't
@@ -856,8 +858,11 @@ recv_upcalls(struct handler *handler)
                               dupcall->key_len, NULL, 0, NULL, 0,
                               &dupcall->ufid, PMD_ID_NULL, NULL);
                 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;
         }
 
@@ -1143,7 +1148,8 @@ upcall_receive(struct upcall *upcall, const struct 
dpif_backer *backer,
                const struct dp_packet *packet, enum dpif_upcall_type type,
                const struct nlattr *userdata, const struct flow *flow,
                const unsigned int mru,
-               const ovs_u128 *ufid, const unsigned pmd_id)
+               const ovs_u128 *ufid, const unsigned pmd_id,
+               char **errorp)
 {
     int error;
 
@@ -1152,7 +1158,8 @@ upcall_receive(struct upcall *upcall, const struct 
dpif_backer *backer,
         return EAGAIN;
     } else if (upcall->type == MISS_UPCALL) {
         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) {
             return error;
         }
@@ -1160,7 +1167,11 @@ upcall_receive(struct upcall *upcall, const struct 
dpif_backer *backer,
         struct ofproto_dpif *ofproto
             = ofproto_dpif_lookup_by_uuid(&upcall->cookie.ofproto_uuid);
         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;
         }
         upcall->ofproto = ofproto;
@@ -1349,7 +1360,7 @@ upcall_cb(const struct dp_packet *packet, const struct 
flow *flow, ovs_u128 *ufi
     atomic_read_relaxed(&enable_megaflows, &megaflow);
 
     error = upcall_receive(&upcall, udpif->backer, packet, type, userdata,
-                           flow, 0, ufid, pmd_id);
+                           flow, 0, ufid, pmd_id, NULL);
     if (error) {
         return error;
     }
@@ -2145,7 +2156,7 @@ xlate_key(struct udpif *udpif, const struct nlattr *key, 
unsigned int len,
     }
 
     error = xlate_lookup(udpif->backer, &ctx->flow, &ofproto, NULL, NULL,
-                         ctx->netflow, &ofp_in_port);
+                         ctx->netflow, &ofp_in_port, NULL);
     if (error) {
         return error;
     }
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 5c3021765..e96697e78 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1592,17 +1592,19 @@ xlate_lookup_ofproto(const struct dpif_backer *backer, 
const struct flow *flow,
  * be taken.
  *
  * 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
 xlate_lookup(const struct dpif_backer *backer, const struct flow *flow,
              struct ofproto_dpif **ofprotop, struct dpif_ipfix **ipfix,
              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;
     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) {
         return ENODEV;
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 2ba90e999..c6cb62cdd 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -205,7 +205,7 @@ struct ofproto_dpif * xlate_lookup_ofproto(const struct 
dpif_backer *,
 int xlate_lookup(const struct dpif_backer *, const struct flow *,
                  struct ofproto_dpif **, struct dpif_ipfix **,
                  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);
 
-- 
2.34.3

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to