[Intel-wired-lan] [PATCH v5 net-next 04/12] net-shapers: implement NL group operation
Paolo Abeni
pabeni at redhat.com
Fri Aug 30 16:48:41 UTC 2024
On 8/30/24 04:04, Jakub Kicinski wrote:
>> +static int __net_shaper_group(struct net_shaper_binding *binding,
>> + int leaves_count,
>> + const struct net_shaper_handle *leaves_handles,
>> + struct net_shaper_info *leaves,
>> + struct net_shaper_handle *node_handle,
>> + struct net_shaper_info *node,
>> + struct netlink_ext_ack *extack)
>> +{
>> + const struct net_shaper_ops *ops = net_shaper_binding_ops(binding);
>> + struct net_shaper_info *parent = NULL;
>> + struct net_shaper_handle leaf_handle;
>> + int i, ret;
>> +
>> + if (node_handle->scope == NET_SHAPER_SCOPE_NODE) {
>> + if (node_handle->id != NET_SHAPER_ID_UNSPEC &&
>> + !net_shaper_cache_lookup(binding, node_handle)) {
>> + NL_SET_ERR_MSG_FMT(extack, "Node shaper %d:%d does not exists",
>> + node_handle->scope, node_handle->id);
>
> BAD_ATTR would do?
We can reach here from the delete() op (next patch), there will be no
paired attribute is such case. Even for the group() operation it will
need to push here towards several callers additional context to identify
the attribute, it should be quite ugly, can we keep with ERR_MSG_FMT here?
>> + if (ret < 0)
>> + goto free_shapers;
>> +
>> + ret = net_shaper_group_send_reply(info, &node_handle);
>> + if (ret) {
>> + /* Error on reply is not fatal to avoid rollback a successful
>> + * configuration.
>
> Slight issues with the grammar here, but I think it should be fatal.
> The sender will most likely block until they get a response.
> Not to mention that the caller will not know what the handle
> we allocated is.
You mean we should return a negative error code, and _not_ that we
should additionally attempt a rollback, right? The rollback will be very
difficult at best: at this point destructive action have taken place.
Thanks,
Paolo
More information about the Intel-wired-lan
mailing list