| Message ID | 20171127215328.1947-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] netdev-tc-offloads: Use customary types for buffer. | expand |
> On Nov 27, 2017, at 1:53 PM, Ben Pfaff <blp@ovn.org> wrote: > > This function uses local array set_buff[] to store Netlink attributes. > It declares set_buff as an array of character pointers, which is a strange > type for a buffer of non-character-pointer objects. In OVS it is > customary to use an ofpbuf with a stub of uint64_t objecs (to ensure > proper alignment, otherwise uint8_t would be more usual). This commit > changes to that more usual form. > > Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Justin Pettit <jpettit@ovn.org> --Justin
On Thu, Dec 21, 2017 at 04:39:04PM -0800, Justin Pettit wrote: > > > > On Nov 27, 2017, at 1:53 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > This function uses local array set_buff[] to store Netlink attributes. > > It declares set_buff as an array of character pointers, which is a strange > > type for a buffer of non-character-pointer objects. In OVS it is > > customary to use an ofpbuf with a stub of uint64_t objecs (to ensure > > proper alignment, otherwise uint8_t would be more usual). This commit > > changes to that more usual form. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > Acked-by: Justin Pettit <jpettit@ovn.org> Thanks, applied to master.
diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c index 1f77b5595594..9364d94f05ef 100644 --- a/lib/netdev-tc-offloads.c +++ b/lib/netdev-tc-offloads.c @@ -581,18 +581,17 @@ parse_put_flow_set_masked_action(struct tc_flower *flower, bool hasmask) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); - char *set_buff[128], *set_data, *set_mask; + uint64_t set_stub[1024 / 8]; + struct ofpbuf set_buf = OFPBUF_STUB_INITIALIZER(set_stub); + char *set_data, *set_mask; char *key = (char *) &flower->rewrite.key; char *mask = (char *) &flower->rewrite.mask; const struct nlattr *attr; int i, j, type; size_t size; - ovs_assert(set_len <= 128); - /* copy so we can set attr mask to 0 for used ovs key struct members */ - memcpy(set_buff, set, set_len); - attr = (struct nlattr *) set_buff; + attr = ofpbuf_put(&set_buf, set, set_len); type = nl_attr_type(attr); size = nl_attr_get_size(attr) / 2; @@ -602,6 +601,7 @@ parse_put_flow_set_masked_action(struct tc_flower *flower, if (type >= ARRAY_SIZE(set_flower_map) || !set_flower_map[type][0].size) { VLOG_DBG_RL(&rl, "unsupported set action type: %d", type); + ofpbuf_uninit(&set_buf); return EOPNOTSUPP; } @@ -634,9 +634,11 @@ parse_put_flow_set_masked_action(struct tc_flower *flower, if (hasmask && !is_all_zeros(set_mask, size)) { VLOG_DBG_RL(&rl, "unsupported sub attribute of set action type %d", type); + ofpbuf_uninit(&set_buf); return EOPNOTSUPP; } + ofpbuf_uninit(&set_buf); return 0; }
This function uses local array set_buff[] to store Netlink attributes. It declares set_buff as an array of character pointers, which is a strange type for a buffer of non-character-pointer objects. In OVS it is customary to use an ofpbuf with a stub of uint64_t objecs (to ensure proper alignment, otherwise uint8_t would be more usual). This commit changes to that more usual form. Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/netdev-tc-offloads.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)