netdev - Re: [PATCH net-next 7/7] cxgb4: add support for drop and redirect actions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Sep 2016 08:17:08 -0700
From:   John Fastabend <john.fastabend@...il.com>
To:     Jiri Pirko <jiri@...nulli.us>,
        Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>,
        Jamal Hadi Salim <jhs@...atatu.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        hariprasad@...lsio.com, leedom@...lsio.com, nirranjan@...lsio.com,
        indranil@...lsio.com
Subject: Re: [PATCH net-next 7/7] cxgb4: add support for drop and redirect
 actions

On 16-09-12 01:52 AM, Jiri Pirko wrote:
> Mon, Sep 12, 2016 at 10:12:40AM CEST, rahul.lakkireddy@...lsio.com wrote:
>> Add support for dropping matched packets in hardware.  Also add support
>> for re-directing matched packets to a specified port in hardware.
>>
>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@...lsio.com>
>> Signed-off-by: Hariprasad Shenai <hariprasad@...lsio.com>
>> ---

[...]

>>
>> +/* Fill ch_filter_specification with parsed action. */
>> +static int fill_action_fields(struct adapter *adap,
>> +			      struct ch_filter_specification *fs,
>> +			      struct tc_cls_u32_offload *cls)
>> +{
>> +	const struct tc_action *a;
>> +	struct tcf_exts *exts;
>> +	LIST_HEAD(actions);
>> +	unsigned int num_actions = 0;
>> +	bool found = false;
>> +
>> +	exts = cls->knode.exts;
>> +	if (tc_no_actions(exts))
>> +		return -EINVAL;
>> +
>> +	tcf_exts_to_list(exts, &actions);
>> +	list_for_each_entry(a, &actions, list) {
>> +		/* Don't allow more than one action per rule. */
>> +		if (num_actions)
>> +			return -EINVAL;
> 
> 
> Looking at this, unrelated to this patch, we really need some advanced
> reporting to user about what went wrong. Otherwise he's playing a
> guessing game.
> 

+1 my recommendation to new users has been to annotate or read the
kernel source when they get errors which is obviously a failure on
our part to build usable error messages.

Note its not even really related to hardware offload its bad just
with software use case and the hardware offloads make it a bit more
mysterious when an error is returned.

Maybe Jamal can add it to his tc-workshop at netdev conference so
we can get some consensus about how to do this.

.John