On 27/03/2018 01:58, Jason Gunthorpe wrote:
On Tue, Mar 20, 2018 at 03:01:36PM +0200, Leon Romanovsky wrote:
+static int (*flow_action_esp_keymat_validate[])(union ib_flow_action_attrs_esp_keymats *keymat) = {
+ [IB_UVERBS_FLOW_ACTION_ESP_KEYMAT_AES_GCM] = validate_flow_action_esp_keymat_aes_gcm,
+};
Should be
static int (* const flow_action_esp_keymat_validate[])(union ib_flow_action_attrs_esp_keymats *keymat) = {
To get in rodata
Ditto for flow_action_esp_replay_validate
Ok
+static struct uverbs_attr_spec uverbs_flow_action_esp_keymat[] = {
+ [IB_UVERBS_FLOW_ACTION_ESP_KEYMAT_AES_GCM] = {
+ .ptr = {
+ .type = UVERBS_ATTR_TYPE_PTR_IN,
+ UVERBS_ATTR_TYPE(struct ib_uverbs_flow_action_esp_keymat_aes_gcm),
+ .flags = UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO,
+ },
+ },
+};
+
+static struct uverbs_attr_spec uverbs_flow_action_esp_replay[] = {
+ [IB_UVERBS_FLOW_ACTION_ESP_REPLAY_BMP] = {
+ .ptr = {
+ .type = UVERBS_ATTR_TYPE_PTR_IN,
+ UVERBS_ATTR_STRUCT(struct ib_uverbs_flow_action_esp_replay_bmp, size),
+ .flags = UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO,
+ }
+ },
+};
These should be 'static const struct' too
But that needs this change in include/rdma/uverbs_ioctl.h
@@ -106,7 +106,7 @@ struct uverbs_attr_spec {
* contained in the ids array. Currently only PTR_IN
* attributes are supported in the ids array.
*/
- struct uverbs_attr_spec *ids;
+ const struct uverbs_attr_spec *ids;
} enum_def;
};
};
Sure
+struct ib_uverbs_flow_action_esp_keymat_aes_gcm {
+ __aligned_u64 iv;
Padding here
+ __u32 iv_algo; /* Use enum ib_uverbs_flow_action_esp_keymat_aes_gcm_iv_algo */
+
+ __u32 salt;
+ __u32 icv_len;
+
+ __u32 key_len;
+ __u32 aes_key[256 / 32];
and implicit padding at the end here
+};
And I thought we agreed to make padding explicit in this uapi
structures?
Please check all the uapi structs with pahole to avoid missing
implicit padding.
Thankfully the __aligned_64 makes this Not a Bug
Why do we need to make all uapi structs u64 aligned in our new uapi?
We don't have the "size in quadwords" like we have in the write() path.
Keeping them 32bit aligned with all u64 fields marked as __aligned_64
seems to suffice. It should prevent the nasty 32bit-vs-64bit (user-space
vs kernel) bugs.
+struct ib_uverbs_flow_action_esp_encap {
+ /* This struct represents a list of pointers to flow_xxxx_filter that
+ * encapsulates the payload in ESP tunnel mode.
+ */
+ RDMA_UAPI_PTR(void *, val_ptr); /* pointer to a flow_xxxx_filter */
+ RDMA_UAPI_PTR(struct ib_uverbs_flow_action_esp_encap *, next_ptr);
+ __u16 len; /* Len of the filter struct val_ptr points to */
+ __u16 type; /* Use flow_spec_type enum */
+};
Also missing trailing implicit padding.
Lets decide if we really want to keep the 64bit alignment restriction
for the new uapi.
Jason
Thanks for the review.
Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
