On 27/03/2018 15:38, Leon Romanovsky wrote:
On Tue, Mar 27, 2018 at 01:39:02PM +0300, Matan Barak wrote:
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.
Alignment to 64bits makes our life much easier: less assumptions and easy reviews.
If alignments were implicit - sure.
But since they're currently explicit (adding reserved fields), it
happened more than once that developers forgot "reserved" fields,
breaking 32bit vs 64bit.
Thanks
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
