[ovs-dev] [RFC PATCH v2 02/10] openvswitch.h: add vport to ovs_action_push_tnl.
Stokes, Ian
ian.stokes at intel.com
Wed Nov 1 16:14:23 UTC 2017
> On Fri, Aug 25, 2017 at 05:40:24PM +0100, Ian Stokes wrote:
> > Upon callback for building/pushing a packet header when encapsulating
> > for tunneling a reference to the vport in question is required to
> > access associated devices such as cryptodevs. This patch adds this
> > pointer and will enable future work with cryptodevs that are
> > associated with a vport.
> >
> > Signed-off-by: Ian Stokes <ian.stokes at intel.com>
> > ---
> > datapath/linux/compat/include/linux/openvswitch.h | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index bc6c94b..afa7faf 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -723,6 +723,8 @@ struct ovs_action_hash {
> > * @tnl_port: To identify tunnel port to pass header info.
> > * @out_port: Physical port to send encapsulated packet.
> > * @header_len: Length of the header to be pushed.
> > + * @dev: Pointer to vport so that the cryptodev parameters associated
> > + with the
> > + * vport can be accessed at the callback function.
> > * @tnl_type: This is only required to format this header. Otherwise
> > * ODP layer can not parse %header.
> > * @header: Partial header for the tunnel. Tunnel push action can use
> > @@ -732,6 +734,7 @@ struct ovs_action_push_tnl {
> > odp_port_t tnl_port;
> > odp_port_t out_port;
> > uint32_t header_len;
> > + struct netdev_vport *dev;
> > uint32_t tnl_type; /* For logging. */
> > uint32_t header[TNL_PUSH_HEADER_SIZE / 4]; };
>
> Maybe this is safe for some reason, but I worry that there's the
> possibility of a use-after-free error. Is 'dev' supposed to hold a
> reference to the netdev (with netdev_ref())? If so, it would be good to
> document that in the comment.
Sure, the purpose of dev here is to allow access to the ipsec options such as security association info (cipher/authentication type, keys etc.) associated with the vport to be referenced during the build_header phase of processing. For example this info would be required to know the length of the initialization vector for the packet header. I'm open to a better way of this info being shared to the build header stage for the vport but the only reference passed was a pointer to ovs_action_push_tnl struct I just want to get an opinion on using it this way.
This was a quick way to enable this for RFC purposes, so there could be a use-after-free error (I didn't hit it in testing myself) but I'll look at it again.
Thanks
Ian
More information about the dev
mailing list