On Wed, Feb 15, 2017 at 03:44:30PM +0200, Roi Dayan wrote:
>
>
> On 14/02/2017 01:55, Chandran, Sugesh wrote:
> >
> >
> > Regards
> > _Sugesh
> >
> >
> > > -----Original Message-----
> > > From: Roi Dayan [mailto:r...@mellanox.com]
> > > Sent: Wednesday, February 8, 2017 3:29 PM
> > > To: d...@openvswitch.org
> > > Cc: Paul Blakey <pa...@mellanox.com>; Or Gerlitz
> > > <ogerl...@mellanox.com>; Hadar Hen Zion <had...@mellanox.com>; Shahar
> > > Klein <shah...@mellanox.com>; Mark Bloch <ma...@mellanox.com>; Rony
> > > Efraim <ro...@mellanox.com>; Fastabend, John R
> > > <john.r.fastab...@intel.com>; Joe Stringer <j...@ovn.org>; Andy
> > > Gospodarek <a...@greyhouse.net>; Lance Richardson
> > > <lrich...@redhat.com>; Marcelo Ricardo Leitner <mleit...@redhat.com>;
> > > Simon Horman <simon.hor...@netronome.com>; Jiri Pirko
> > > <j...@mellanox.com>; Chandran, Sugesh <sugesh.chand...@intel.com>
> > > Subject: [PATCH ovs V3 12/25] dpif-netlink: Use netdev flow put api to
> > > insert
> > > a flow
> > >
> > > From: Paul Blakey <pa...@mellanox.com>
> > >
> > > +
> > > + // XXX: missing ofpbuf_uninit?
> > > + return 0;
> > > +}
> > > +
> > > +static int
> > > +parse_flow_put(struct dpif_netlink *dpif, struct dpif_flow_put *put) {
> > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> > > + struct match match;
> > > + odp_port_t in_port;
> > > + const struct nlattr *nla;
> > > + size_t left;
> > > + int outputs = 0;
> > > + struct netdev *dev;
> > > + struct offload_info info;
> > > + ovs_be16 dst_port = 0;
> > > + int err = 0;
> > > +
> > > + if (put->flags & DPIF_FP_PROBE) {
> > > + return EOPNOTSUPP;
> > > + }
> > > +
> > > + err = parse_key_and_mask_to_match(put->key, put->key_len, put-
> > > > mask,
> > > + put->mask_len, &match);
> > > + if (err) {
> > > + return err;
> > > + }
> > > +
> > > + /* Get tunnel dst port and count outputs */
> > > + NL_ATTR_FOR_EACH(nla, left, put->actions, put->actions_len) {
> > > + if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
> > > + const struct netdev_tunnel_config *tnl_cfg;
> > > + struct netdev *outdev;
> > > + odp_port_t out_port;
> > > +
> > [Sugesh] Just wondering , is it the right place to do these checks. In
> > fact these are limited by the hardware(Correct me if I am wrong here). So
> > this has to under the specific netdev_flow_put than here?
>
> right. it was done here for convenience. should probably move it. thanks.
>
> > > + outputs++;
> > > + if (outputs > 1) {
> > > + VLOG_WARN_RL(&rl, "offloading multiple ports isn't
> > > supported");
Also, you may want to consider reducing the log level of this message.
Seems I'm hitting it for all broadcasts on the subnet.
2017-02-17T17:28:35.025Z|03238|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:28:45.437Z|03239|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:28:50.315Z|03240|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:28:55.376Z|00649|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:29:17.834Z|03241|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:29:39.952Z|00650|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:29:54.321Z|03242|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:29:59.498Z|03243|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:30:14.300Z|03244|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:30:22.338Z|03245|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:30:24.896Z|00651|dpif_netlink(handler20)|WARN|offloading
multiple ports isn't supported
2017-02-17T17:30:43.072Z|03246|dpif_netlink(handler14)|WARN|offloading
multiple ports isn't supported
....
for flows like:
recirc_id(0),in_port(2),eth(src=XX:XX:XX:cb:76:af,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0800),ipv4(frag=no),
packets:2, bytes:692, used:8.821s, actions:3,1
And/or you plan to support it?
> > > + err = EOPNOTSUPP;
> > > + goto err_out;
> > > + }
> > > +
> > > + out_port = nl_attr_get_odp_port(nla);
> > > + outdev = netdev_hmap_port_get(out_port, DPIF_HMAP_KEY(&dpif-
> > > > dpif));
> > > + tnl_cfg = netdev_get_tunnel_config(outdev);
> > > + if (tnl_cfg && tnl_cfg->dst_port != 0) {
> > > + dst_port = tnl_cfg->dst_port;
> > > + }
> > > + netdev_close(outdev);
> > > + }
> > > + }
> > > +
> > > +
> > > + info.port_hmap_obj = DPIF_HMAP_KEY(&dpif->dpif);
> > > + info.tp_dst_port = dst_port;
> > > + in_port = match.flow.in_port.odp_port;
> > > + dev = netdev_hmap_port_get(in_port, DPIF_HMAP_KEY(&dpif->dpif));
> > > + err = netdev_flow_put(dev, &match,
> > > + CONST_CAST(struct nlattr *, put->actions),
> > > + put->actions_len, put->stats,
> > > + CONST_CAST(ovs_u128 *, put->ufid), &info);
> > > + netdev_close(dev);
> > > +
> > > + if (!err) {
> > > + if (put->flags & DPIF_FP_MODIFY) {
> > > + struct dpif_op *opp;
> > > + struct dpif_op op;
> > > +
> > > + op.type = DPIF_OP_FLOW_DEL;
> > > + op.u.flow_del.key = put->key;
> > > + op.u.flow_del.key_len = put->key_len;
> > > + op.u.flow_del.ufid = put->ufid;
> > > + op.u.flow_del.pmd_id = put->pmd_id;
> > > + op.u.flow_del.stats = NULL;
> > > + op.u.flow_del.terse = false;
> > > +
> > > + opp = &op;
> > > + dpif_netlink_operate__(dpif, &opp, 1);
> > > + }
> > > + VLOG_DBG("added flow");
> > > + } else if (err != EEXIST) {
> > > + VLOG_ERR_RL(&rl, "failed adding flow: %s", ovs_strerror(err));
> > > + }
> > > +
> > > +err_out:
> > > +
> > > + return err;
> > > +}
> > > +
> > > static void
> > > -dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t
> > > n_ops)
> > > +dbg_print_flow(const struct nlattr *key, size_t key_len,
> > > + const struct nlattr *mask, size_t mask_len,
> > > + const struct nlattr *actions, size_t actions_len,
> > > + const ovs_u128 *ufid,
> > > + const char *op)
> > > +{
> > > + struct ds s;
> > > +
> > > + ds_init(&s);
> > > + ds_put_cstr(&s, op);
> > > + ds_put_cstr(&s, " (");
> > > + odp_format_ufid(ufid, &s);
> > > + ds_put_cstr(&s, ")");
> > > + if (key_len) {
> > > + ds_put_cstr(&s, "\nflow (verbose): ");
> > > + odp_flow_format(key, key_len, mask, mask_len, NULL, &s,
> > > true);
> > > + ds_put_cstr(&s, "\nflow: ");
> > > + odp_flow_format(key, key_len, mask, mask_len, NULL, &s,
> > > false);
> > > + }
> > > + ds_put_cstr(&s, "\nactions: ");
> > > + format_odp_actions(&s, actions, actions_len);
> > > + VLOG_DBG("\n%s", ds_cstr(&s));
> > > + ds_destroy(&s);
> > > +}
> > > +
> > > +static int
> > > +try_send_to_netdev(struct dpif_netlink *dpif, struct dpif_op *op)
> > > {
> > > - struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> > > + switch (op->type) {
> > > + case DPIF_OP_FLOW_PUT: {
> > > + struct dpif_flow_put *put = &op->u.flow_put;
> > > +
> > > + if (!put->ufid) {
> > > + break;
> > > + }
> > > + dbg_print_flow(put->key, put->key_len, put->mask, put->mask_len,
> > > + put->actions, put->actions_len, put->ufid, "PUT");
> > > + return parse_flow_put(dpif, put);
> > > + }
> > > + case DPIF_OP_FLOW_DEL:
> > > + case DPIF_OP_FLOW_GET:
> > > + case DPIF_OP_EXECUTE:
> > > + default:
> > > + break;
> > > + }
> > > + return EOPNOTSUPP;
> > > +}
> > >
> > > +static void
> > > +dpif_netlink_operate_chunks(struct dpif_netlink *dpif, struct dpif_op
> > > **ops,
> > > + size_t n_ops) {
> > > while (n_ops > 0) {
> > > size_t chunk = dpif_netlink_operate__(dpif, ops, n_ops);
> > > +
> > > ops += chunk;
> > > n_ops -= chunk;
> > > }
> > > }
> > >
> > > +static void
> > > +dpif_netlink_operate(struct dpif *dpif_, struct dpif_op **ops, size_t
> > > +n_ops) {
> > > + struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> > > + struct dpif_op *new_ops[OPERATE_MAX_OPS];
> > > + int count = 0;
> > > + int i = 0;
> > > + int err = 0;
> > > +
> > > + if (netdev_flow_api_enabled) {
> > > + while (n_ops > 0) {
> > > + count = 0;
> > > +
> > > + while (n_ops > 0 && count < OPERATE_MAX_OPS) {
> > > + struct dpif_op *op = ops[i++];
> > > +
> > > + err = try_send_to_netdev(dpif, op);
> > > + if (err && err != EEXIST) {
> > > + new_ops[count++] = op;
> > > + } else {
> > > + op->error = err;
> > > + }
> > > +
> > > + n_ops--;
> > > + }
> > > +
> > > + dpif_netlink_operate_chunks(dpif, new_ops, count);
> > > + }
> > > +
> > > + return;
> > > + }
> > > +
> > > + dpif_netlink_operate_chunks(dpif, ops, n_ops); }
> > > +
> > > #if _WIN32
> > > static void
> > > dpif_netlink_handler_uninit(struct dpif_handler *handler) diff --git
> > > a/lib/odp-util.c b/lib/odp-util.c index 4106738..274d2ee 100644
> > > --- a/lib/odp-util.c
> > > +++ b/lib/odp-util.c
> > > @@ -41,6 +41,7 @@
> > > #include "util.h"
> > > #include "uuid.h"
> > > #include "openvswitch/vlog.h"
> > > +#include "openvswitch/match.h"
> > >
> > > VLOG_DEFINE_THIS_MODULE(odp_util);
> > >
> > > @@ -5294,6 +5295,58 @@ odp_flow_key_to_mask(const struct nlattr
> > > *mask_key, size_t mask_key_len,
> > > }
> > > }
> > >
> > > +/* Converts the netlink formated key/mask to match.
> > > + * Fails if odp_flow_key_from_key/mask and odp_flow_key_key/mask
> > > + * disagree on the acceptable form of flow */ int
> > > +parse_key_and_mask_to_match(const struct nlattr *key, size_t key_len,
> > > + const struct nlattr *mask, size_t mask_len,
> > > + struct match *match) {
> > > + enum odp_key_fitness fitness;
> > > +
> > > + fitness = odp_flow_key_to_flow(key, key_len, &match->flow);
> > > + if (fitness) {
> > > + /* This should not happen: it indicates that
> > > odp_flow_key_from_flow()
> > > + * and odp_flow_key_to_flow() disagree on the acceptable form of
> > > a
> > > + * flow. Log the problem as an error, with enough details to
> > > enable
> > > + * debugging. */
> > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > > +
> > > + if (!VLOG_DROP_ERR(&rl)) {
> > > + struct ds s;
> > > +
> > > + ds_init(&s);
> > > + odp_flow_format(key, key_len, NULL, 0, NULL, &s, true);
> > > + VLOG_ERR("internal error parsing flow key %s", ds_cstr(&s));
> > > + ds_destroy(&s);
> > > + }
> > > +
> > > + return EINVAL;
> > > + }
> > > +
> > > + fitness = odp_flow_key_to_mask(mask, mask_len, &match->wc,
> > > &match->flow);
> > > + if (fitness) {
> > > + /* This should not happen: it indicates that
> > > + * odp_flow_key_from_mask() and odp_flow_key_to_mask()
> > > + * disagree on the acceptable form of a mask. Log the problem
> > > + * as an error, with enough details to enable debugging. */
> > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> > > +
> > > + if (!VLOG_DROP_ERR(&rl)) {
> > > + struct ds s;
> > > +
> > > + VLOG_ERR("internal error parsing flow mask %s (%s)",
> > > + ds_cstr(&s), odp_key_fitness_to_string(fitness));
> > > + ds_destroy(&s);
> > > + }
> > > +
> > > + return EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /* Returns 'fitness' as a string, for use in debug messages. */ const
> > > char *
> > > odp_key_fitness_to_string(enum odp_key_fitness fitness) diff --git
> > > a/lib/odp-util.h b/lib/odp-util.h index 42011bc..3f59ab7 100644
> > > --- a/lib/odp-util.h
> > > +++ b/lib/odp-util.h
> > > @@ -236,6 +236,9 @@ enum odp_key_fitness
> > > odp_flow_key_to_mask(const struct nlattr *mask_key,
> > > size_t mask_key_len,
> > > struct flow_wildcards *mask,
> > > const struct flow *flow);
> > > +int parse_key_and_mask_to_match(const struct nlattr *key, size_t
> > > key_len,
> > > + const struct nlattr *mask, size_t
> > > mask_len,
> > > + struct match *match);
> > >
> > > const char *odp_key_fitness_to_string(enum odp_key_fitness);
> > >
> > > --
> > > 2.7.4
> >
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
