From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Mi <chrism@mellanox.com>
Subject: Re: [patch iproute2 v4 2/3] tc: Add -bs option to batch mode
Date: Wed, 3 Jan 2018 10:44:44 +0800
Message-ID: <6c660465-2a32-c490-45e1-15e4513ce92a@mellanox.com>
References: <20180102142804.27145-1-chrism@mellanox.com>
 <20180102142804.27145-3-chrism@mellanox.com>
 <20180102200425.GA725@localhost.localdomain>
Mime-Version: 1.0
Content-Type: text/plain; charset=gbk; format=flowed
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, gerlitz.or@gmail.com,
        stephen@networkplumber.org, dsahern@gmail.com
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-eopbgr00064.outbound.protection.outlook.com ([40.107.0.64]:49012
        "EHLO EUR02-AM5-obe.outbound.protection.outlook.com"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1751052AbeACCo5 (ORCPT <rfc822;netdev@vger.kernel.org>);
        Tue, 2 Jan 2018 21:44:57 -0500
In-Reply-To: <20180102200425.GA725@localhost.localdomain>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>


> On Tue, Jan 02, 2018 at 11:28:03PM +0900, Chris Mi wrote:
>> @@ -240,23 +244,49 @@ static int batch(const char *name)
>>   	}
>>   
>>   	cmdlineno = 0;
>> -	while (getcmdline(&line, &len, stdin) != -1) {
>> +	if (getcmdline(&line, &len, stdin) == -1)
>> +		goto Exit;
>> +	do {
>>   		char *largv[100];
>>   		int largc;
>>   
>> +		if (getcmdline(&line2, &len, stdin) == -1)
>> +			lastline = true;
>> +
>>   		largc = makeargs(line, largv, 100);
>>   		if (largc == 0)
>>   			continue;	/* blank line */
> If it reads a new line, it won't process anything else after it
> because line won't get updated.
Indeed. Thanks for catching it. After fixing it, I find that it only 
works if the blank line is
in the beginning or middle. If the blank line is in the end, we may lose 
at most batchsize - 1 rules.
It is not easy to fix it. I think this issue is trivial. It is not worth 
to make the code complex to fix it.
So I describe this limitation in the man page.

-Chris
>
>    Marcelo
>
>>   
>> -		if (do_cmd(largc, largv)) {
>> -			fprintf(stderr, "Command failed %s:%d\n", name, cmdlineno);
>> +		line = line2;
>> +		line2 = NULL;
>> +		len = 0;
>> +
>> +		/*
>> +		 * In batch mode, if we haven't accumulated enough commands
>> +		 * and this is not the last command, don't send the message
>> +		 * immediately.
>> +		 */
>> +		if (batch_size > 1 && msg_iov_index + 1 != batch_size
>> +		    && !lastline)
>> +			send = false;
>> +		else
>> +			send = true;
>> +
>> +		ret = do_cmd(largc, largv, batch_size, msg_iov_index++, send);
>> +		if (ret < 0) {
>> +			fprintf(stderr, "Command failed %s:%d\n", name,
>> +				cmdlineno);
>>   			ret = 1;
>>   			if (!force)
>>   				break;
>>   		}
>> -	}
>> -	if (line)
>> -		free(line);
>> +		msg_iov_index %= batch_size;
>> +	} while (!lastline);
>> +
>> +	free_filter_reqs();
>> +	free_action_reqs();
>> +Exit:
>> +	free(line);
>>   
>>   	rtnl_close(&rth);
>>   	return ret;