On Tue, Mar 19, 2019 at 05:56:12PM +0800, wenxu wrote:
>
> On 3/19/2019 4:21 PM, Simon Horman wrote:
> > Hi Wenxu,
> >
> > I think that the error you are seeing is the expected behaviour whereby
> > netdev_tc_flow_put() returns EOPNOTSUPP when it receives a flow
> > whose offload is not supported. And it seems to me that is the
> > correct place for this to be handled.
> >
> > I am referring to the following code around line 1555 of
> > lib/netdev-tc-offloads.c
> >
> > } else {
> > VLOG_DBG_RL(&rl, "unsupported put action type: %d",
> > nl_attr_type(nla));
> > return EOPNOTSUPP;
> > }
> >
> > I do, however, wonder if the log message you report should be of level info
> > rather than error in the case where the error is EOPNOTSUPP: this isn't an
> > error in the sense that the system is malfunctioning but rather lower
> > layers are reporting that a flow can't be offloaded because it is not
> > supported which is a part of the design of support for offloads.
>
> It maybe better for depreciate the log message for some EOPNOTSUPP case
> (USERSPACE action)?
>
> It is not an err case the the lower layer is not supported. It indeed can't
> support by hw. The user will not
>
> care about this and don't expect receive the logs. On the online system, it
> will lead many unused log for user.
Hi Wenxu,
I think there is some value in logging the message at a lower priority,
say debug rather than err, and to allow users to see the run-time behaviour
if their log filter is setup accordingly.
Looking at the code it seems that ENOSPC may also a good candidate for this
kind of treatment. Something like the following (compile tested only).
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 00538e5e47d9..92dd003452a9 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2067,6 +2067,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
VLOG_DBG("added flow");
} else if (err != EEXIST) {
+ enum vlog_level level;
struct netdev *oor_netdev = NULL;
if (err == ENOSPC && netdev_is_offload_rebalance_policy_enabled()) {
/*
@@ -2082,8 +2083,10 @@ parse_flow_put(struct dpif_netlink *dpif, struct
dpif_flow_put *put)
}
netdev_set_hw_info(oor_netdev, HW_INFO_TYPE_OOR, true);
}
- VLOG_ERR_RL(&rl, "failed to offload flow: %s: %s", ovs_strerror(err),
- (oor_netdev ? oor_netdev->name : dev->name));
+ level = (err == ENOSPC || err == EOPNOTSUPP) ? VLL_DBG : VLL_ERR;
+ VLOG_RL(&rl, level, "failed to offload flow: %s: %s",
+ ovs_strerror(err),
+ (oor_netdev ? oor_netdev->name : dev->name));
}
out:
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
