2016-08-09 10:20 GMT-07:00 Ian Stokes <ian.sto...@intel.com>: > When egress policer is set as a QoS type for a port, an error may occur > during > setup if incorrect parameters are used for the rte_meter. If this occurs > the egress policer construct and set functions should free any allocated > memory relevant to the policer and set the QoS configuration pointer to > null. The netdev_dpdk_set_qos function should check the error value > returned > for any QoS construct/set calls with an assertion to avoid segfault. > Also this commit modifies egress_policer_qos_set() to correctly lock the > QoS > spinlock while the egress policer configuration is updated to avoid > segfault. > > Signed-off-by: Ian Stokes <ian.sto...@intel.com> > --- > v2 > * netdev-dpdk.c > - Simplify assertion in netdev_dpdk_set_qos() to check that no error > has been returned and that a QoS configuration exists before checking > and logging an error. > - Use rte_strerror in netdev_dpdk_set_qos() when logging error for a > textual representation. > - Align VLOG message for correct formatting in netdev_dpdk_set_qos(). > - egress_policer_qos_construct() now returns positive error. > - egress_policer_qos_set() now return positive error. > - Document addition of spinlock in egress_policer_qos_set() in commit > message. > --- > lib/netdev-dpdk.c | 30 ++++++++++++++++++++++++++++-- > 1 files changed, 28 insertions(+), 2 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index bf3a898..f37130e 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -2731,11 +2731,16 @@ netdev_dpdk_set_qos(struct netdev *netdev, > > /* Install new QoS configuration. */ > error = new_ops->qos_construct(netdev, details); > - ovs_assert((error == 0) == (dev->qos_conf != NULL)); > } > } else { > error = new_ops->qos_construct(netdev, details); > - ovs_assert((error == 0) == (dev->qos_conf != NULL)); > + } > + > + ovs_assert((error == 0) == (dev->qos_conf != NULL)); > + if (error) { > + VLOG_ERR("Failed to set QoS type %s on port %s, returned error: > %s", > + type, netdev->name, rte_strerror(-error)); > + ovs_assert(dev->qos_conf == NULL); >
This assert should be unnecessary, given the assert above the if. I removed it and I pushed this to master, thanks _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev