netdev - Re: [next-queue PATCH v2 8/8] igb: Add support for adding offloaded clsflower filters

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]

Message-ID: <20180306112806.6fa39148@cakuba.netronome.com>
Date:   Tue, 6 Mar 2018 11:28:06 -0800
From:   Jakub Kicinski <kubakici@...pl>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc:     intel-wired-lan@...ts.osuosl.org, jeffrey.t.kirsher@...el.com,
        netdev@...r.kernel.org, jesus.sanchez-palencia@...el.com
Subject: Re: [next-queue PATCH v2 8/8] igb: Add support for adding offloaded
 clsflower filters

On Tue, 06 Mar 2018 11:08:26 -0800, Vinicius Costa Gomes wrote:
> >> +static int igb_parse_cls_flower(struct igb_adapter *adapter,
> >> +				struct tc_cls_flower_offload *f,
> >> +				int traffic_class,
> >> +				struct igb_nfc_filter *input)
> >> +{
> >> +	if (f->dissector->used_keys &
> >> +	    ~(BIT(FLOW_DISSECTOR_KEY_BASIC) |
> >> +	      BIT(FLOW_DISSECTOR_KEY_CONTROL) |
> >> +	      BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS) |
> >> +	      BIT(FLOW_DISSECTOR_KEY_VLAN))) {
> >> +		dev_err(&adapter->pdev->dev, "Unsupported key used: 0x%x\n",
> >> +			f->dissector->used_keys);  
> >
> > This will probably trigger for opportunistic offload (non-skip_sw) and
> > confuse users.
> 
> I see your point. I will change to 'dev_warn()', it should not surprise
> users too much, right?

Yes, I think that would be fine, other drivers are doing that already.

IMHO best approach is to not print anything unless skip-sw is set.  If
you used netlink's extack capability exclusively it would "just work".
Extack will only carry the error in case offload is requested.  Could
you consider using extack or do you have a preference to print into the
logs?  You could do both, too.

You can get to extack via f->common->extack.