[ovs-dev] [PATCH] dpif: Remove support for multiple queues per port.
Ben Pfaff
blp at ovn.org
Wed Sep 26 22:57:37 UTC 2018
Thanks, applied to master.
On Wed, Sep 26, 2018 at 03:29:16PM -0700, Yifeng Sun wrote:
> It looks good to me, and testing shows no problem. Thanks.
>
> Tested-by: Yifeng Sun <pkusunyifeng at gmail.com>
> Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>
>
> On Tue, Sep 25, 2018 at 3:14 PM Ben Pfaff <blp at ovn.org> wrote:
>
> > Commit 69c51582ff78 ("dpif-netlink: don't allocate per thread netlink
> > sockets") removed dpif-netlink support for multiple queues per port.
> > No remaining dpif provider supports multiple queues per port, so
> > remove infrastructure for the feature.
> >
> > CC: Matteo Croce <mcroce at redhat.com>
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> > lib/dpif-netlink.c | 9 ++++-----
> > lib/dpif-provider.h | 14 ++------------
> > lib/dpif.c | 15 +++------------
> > lib/dpif.h | 15 +--------------
> > ofproto/ofproto-dpif-upcall.c | 7 +++----
> > ofproto/ofproto-dpif-xlate.c | 6 ++----
> > 6 files changed, 15 insertions(+), 51 deletions(-)
> >
> > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> > index 4736d21d4abc..21315033cc66 100644
> > --- a/lib/dpif-netlink.c
> > +++ b/lib/dpif-netlink.c
> > @@ -234,7 +234,7 @@ static bool ovs_tunnels_out_of_tree = true;
> > static int dpif_netlink_init(void);
> > static int open_dpif(const struct dpif_netlink_dp *, struct dpif **);
> > static uint32_t dpif_netlink_port_get_pid(const struct dpif *,
> > - odp_port_t port_no, uint32_t
> > hash);
> > + odp_port_t port_no);
> > static void dpif_netlink_handler_uninit(struct dpif_handler *handler);
> > static int dpif_netlink_refresh_channels(struct dpif_netlink *,
> > uint32_t n_handlers);
> > @@ -991,7 +991,7 @@ dpif_netlink_port_query_by_name(const struct dpif
> > *dpif_, const char *devname,
> >
> > static uint32_t
> > dpif_netlink_port_get_pid__(const struct dpif_netlink *dpif,
> > - odp_port_t port_no, uint32_t hash OVS_UNUSED)
> > + odp_port_t port_no)
> > OVS_REQ_RDLOCK(dpif->upcall_lock)
> > {
> > uint32_t port_idx = odp_to_u32(port_no);
> > @@ -1015,14 +1015,13 @@ dpif_netlink_port_get_pid__(const struct
> > dpif_netlink *dpif,
> > }
> >
> > static uint32_t
> > -dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no,
> > - uint32_t hash)
> > +dpif_netlink_port_get_pid(const struct dpif *dpif_, odp_port_t port_no)
> > {
> > const struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
> > uint32_t ret;
> >
> > fat_rwlock_rdlock(&dpif->upcall_lock);
> > - ret = dpif_netlink_port_get_pid__(dpif, port_no, hash);
> > + ret = dpif_netlink_port_get_pid__(dpif, port_no);
> > fat_rwlock_unlock(&dpif->upcall_lock);
> >
> > return ret;
> > diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> > index debdafc42992..eb3ee50a6320 100644
> > --- a/lib/dpif-provider.h
> > +++ b/lib/dpif-provider.h
> > @@ -191,16 +191,7 @@ struct dpif_class {
> >
> > /* Returns the Netlink PID value to supply in
> > OVS_ACTION_ATTR_USERSPACE
> > * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
> > - * flows whose packets arrived on port 'port_no'. In the case where
> > the
> > - * provider allocates multiple Netlink PIDs to a single port, it may
> > use
> > - * 'hash' to spread load among them. The caller need not use a
> > particular
> > - * hash function; a 5-tuple hash is suitable.
> > - *
> > - * (The datapath implementation might use some different hash
> > function for
> > - * distributing packets received via flow misses among PIDs. This
> > means
> > - * that packets received via flow misses might be reordered relative
> > to
> > - * packets received via userspace actions. This is not ordinarily a
> > - * problem.)
> > + * flows whose packets arrived on port 'port_no'.
> > *
> > * A 'port_no' of UINT32_MAX should be treated as a special case. The
> > * implementation should return a reserved PID, not allocated to any
> > port,
> > @@ -212,8 +203,7 @@ struct dpif_class {
> > *
> > * A dpif provider that doesn't have meaningful Netlink PIDs can use
> > NULL
> > * for this function. This is equivalent to always returning 0. */
> > - uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no,
> > - uint32_t hash);
> > + uint32_t (*port_get_pid)(const struct dpif *dpif, odp_port_t port_no);
> >
> > /* Attempts to begin dumping the ports in a dpif. On success,
> > returns 0
> > * and initializes '*statep' with any data needed for iteration. On
> > diff --git a/lib/dpif.c b/lib/dpif.c
> > index 85cf9000e80b..4697a4dcd519 100644
> > --- a/lib/dpif.c
> > +++ b/lib/dpif.c
> > @@ -737,16 +737,7 @@ dpif_port_query_by_name(const struct dpif *dpif,
> > const char *devname,
> >
> > /* Returns the Netlink PID value to supply in OVS_ACTION_ATTR_USERSPACE
> > * actions as the OVS_USERSPACE_ATTR_PID attribute's value, for use in
> > - * flows whose packets arrived on port 'port_no'. In the case where the
> > - * provider allocates multiple Netlink PIDs to a single port, it may use
> > - * 'hash' to spread load among them. The caller need not use a particular
> > - * hash function; a 5-tuple hash is suitable.
> > - *
> > - * (The datapath implementation might use some different hash function for
> > - * distributing packets received via flow misses among PIDs. This means
> > - * that packets received via flow misses might be reordered relative to
> > - * packets received via userspace actions. This is not ordinarily a
> > - * problem.)
> > + * flows whose packets arrived on port 'port_no'.
> > *
> > * A 'port_no' of ODPP_NONE is a special case: it returns a reserved PID,
> > not
> > * allocated to any port, that the client may use for special purposes.
> > @@ -757,10 +748,10 @@ dpif_port_query_by_name(const struct dpif *dpif,
> > const char *devname,
> > * update all of the flows that it installed that contain
> > * OVS_ACTION_ATTR_USERSPACE actions. */
> > uint32_t
> > -dpif_port_get_pid(const struct dpif *dpif, odp_port_t port_no, uint32_t
> > hash)
> > +dpif_port_get_pid(const struct dpif *dpif, odp_port_t port_no)
> > {
> > return (dpif->dpif_class->port_get_pid
> > - ? (dpif->dpif_class->port_get_pid)(dpif, port_no, hash)
> > + ? (dpif->dpif_class->port_get_pid)(dpif, port_no)
> > : 0);
> > }
> >
> > diff --git a/lib/dpif.h b/lib/dpif.h
> > index 8fdfe5f00db8..1a35cc410ccd 100644
> > --- a/lib/dpif.h
> > +++ b/lib/dpif.h
> > @@ -274,18 +274,6 @@
> > *
> > * - Upcalls that specify the "special" Netlink PID are queued
> > separately.
> > *
> > - * Multiple threads may want to read upcalls simultaneously from a single
> > - * datapath. To support multiple threads well, one extends the above
> > preferred
> > - * behavior:
> > - *
> > - * - Each port has multiple PIDs. The datapath distributes "miss"
> > upcalls
> > - * across the PIDs, ensuring that a given flow is mapped in a stable
> > way
> > - * to a single PID.
> > - *
> > - * - For "action" upcalls, the thread can specify its own Netlink PID
> > or
> > - * other threads' Netlink PID of the same port for offloading purpose
> > - * (e.g. in a "round robin" manner).
> > - *
> > *
> > * Packet Format
> > * =============
> > @@ -470,8 +458,7 @@ int dpif_port_query_by_name(const struct dpif *, const
> > char *devname,
> > struct dpif_port *);
> > int dpif_port_get_name(struct dpif *, odp_port_t port_no,
> > char *name, size_t name_size);
> > -uint32_t dpif_port_get_pid(const struct dpif *, odp_port_t port_no,
> > - uint32_t hash);
> > +uint32_t dpif_port_get_pid(const struct dpif *, odp_port_t port_no);
> >
> > struct dpif_port_dump {
> > const struct dpif *dpif;
> > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> > index 62222079f07c..0cc964a7f3d2 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -1021,7 +1021,6 @@ classify_upcall(enum dpif_upcall_type type, const
> > struct nlattr *userdata,
> > * initialized with at least 128 bytes of space. */
> > static void
> > compose_slow_path(struct udpif *udpif, struct xlate_out *xout,
> > - const struct flow *flow,
> > odp_port_t odp_in_port, ofp_port_t ofp_in_port,
> > struct ofpbuf *buf, uint32_t meter_id,
> > struct uuid *ofproto_uuid)
> > @@ -1038,7 +1037,7 @@ compose_slow_path(struct udpif *udpif, struct
> > xlate_out *xout,
> > port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP)
> > ? ODPP_NONE
> > : odp_in_port;
> > - pid = dpif_port_get_pid(udpif->dpif, port, flow_hash_5tuple(flow, 0));
> > + pid = dpif_port_get_pid(udpif->dpif, port);
> >
> > size_t offset;
> > size_t ac_offset;
> > @@ -1196,7 +1195,7 @@ upcall_xlate(struct udpif *udpif, struct upcall
> > *upcall,
> > odp_actions->data, odp_actions->size);
> > } else {
> > /* upcall->put_actions already initialized by upcall_receive(). */
> > - compose_slow_path(udpif, &upcall->xout, upcall->flow,
> > + compose_slow_path(udpif, &upcall->xout,
> > upcall->flow->in_port.odp_port,
> > upcall->ofp_in_port,
> > &upcall->put_actions,
> > upcall->ofproto->up.slowpath_meter_id,
> > @@ -2155,7 +2154,7 @@ revalidate_ukey__(struct udpif *udpif, const struct
> > udpif_key *ukey,
> > goto exit;
> > }
> >
> > - compose_slow_path(udpif, xoutp, &ctx.flow,
> > ctx.flow.in_port.odp_port,
> > + compose_slow_path(udpif, xoutp, ctx.flow.in_port.odp_port,
> > ofp_in_port, odp_actions,
> > ofproto->up.slowpath_meter_id, &ofproto->uuid);
> > }
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index d0e7c6b9f55d..92d3c8932b63 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -3084,8 +3084,7 @@ compose_sample_action(struct xlate_ctx *ctx,
> >
> > odp_port_t odp_port = ofp_port_to_odp_port(
> > ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
> > - uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port,
> > - flow_hash_5tuple(&ctx->xin->flow,
> > 0));
> > + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port);
> > size_t cookie_offset = odp_put_userspace_action(pid, cookie,
> > sizeof *cookie,
> > tunnel_out_port,
> > @@ -4637,8 +4636,7 @@ put_controller_user_action(struct xlate_ctx *ctx,
> >
> > odp_port_t odp_port = ofp_port_to_odp_port(ctx->xbridge,
> >
> > ctx->xin->flow.in_port.ofp_port);
> > - uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port,
> > - flow_hash_5tuple(&ctx->xin->flow,
> > 0));
> > + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port);
> > odp_put_userspace_action(pid, &cookie, sizeof cookie, ODPP_NONE,
> > false, ctx->odp_actions);
> > }
> > --
> > 2.16.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
More information about the dev
mailing list