> 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 <[email protected]> > > --- > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
