[LTP] [PATCH 2/3] tst_netdevice: Add helper functions for qdisc and filter management

[Martin Doucha]

On 28. 07. 23 9:53, Cyril Hrubis wrote:
> Hi!
>> --- a/lib/tst_netdevice.c
>> +++ b/lib/tst_netdevice.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/veth.h>
>>   #include <sys/socket.h>
>>   #include <net/if.h>
>> +#include <linux/pkt_sched.h>
>>   #include "lapi/rtnetlink.h"
>>   
>>   #define TST_NO_DEFAULT_MAIN
>> @@ -518,3 +519,116 @@ int tst_netdev_remove_route_inet(const char *file, const int lineno,
>>   	return modify_route_inet(file, lineno, RTM_DELROUTE, 0, ifname,
>>   		srcaddr, srcprefix, dstaddr, dstprefix, gateway);
>>   }
>> +
>> +static int modify_qdisc(const char *file, const int lineno, const char *object,
>> +	unsigned int action, unsigned int nl_flags, const char *ifname,
>> +	unsigned int family, unsigned int parent, unsigned int handle,
>> +	unsigned int info, const char *qd_kind,
>> +	const struct tst_rtnl_attr_list *config)
>> +{
>> +	struct tst_rtnl_context *ctx;
>> +	int ret;
>> +	struct tcmsg msg = {
>> +		.tcm_family = family,
>> +		.tcm_handle = handle,
>> +		.tcm_parent = parent,
>> +		.tcm_info = info
>> +	};
>> +
>> +	if (!qd_kind) {
>> +		tst_brk_(file, lineno, TBROK,
>> +			"Queueing discipline name required");
>> +		return 0;
>> +	}
>> +
>> +	if (ifname) {
>> +		msg.tcm_ifindex = tst_netdev_index_by_name(file, lineno,
>> +			ifname);
>> +
>> +		if (msg.tcm_ifindex < 0) {
>> +			tst_brk_(file, lineno, TBROK, "Interface %s not found",
>> +				ifname);
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	ctx = create_request(file, lineno, action, nl_flags, &msg, sizeof(msg));
>> +
>> +	if (!ctx)
>> +		return 0;
>> +
>> +	if (!tst_rtnl_add_attr_string(file, lineno, ctx, TCA_KIND, qd_kind)) {
>> +		tst_rtnl_destroy_context(file, lineno, ctx);
>> +		return 0;
>> +	}
>> +
>> +	if (config && !tst_rtnl_add_attr_list(file, lineno, ctx, config)) {
>> +		tst_rtnl_destroy_context(file, lineno, ctx);
>> +		return 0;
>> +	}
> 
> 
> Here as well, shouldn't we tst_brk_() consistently if we fail to prepare
> the context?

Same as in the previous patch. If we get into the failure branch here, 
tst_brk_() was already called somewhere in create_request() or 
tst_rtnl_add_attr_*() and it didn't terminate the process because we're 
in the cleanup phase.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic