netdev - Re: [RFC PATCH 4/4] bridge: make hw offload conditional on bridge and bridge port offload flags

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:	Fri, 21 Nov 2014 23:30:54 +0000
From:	Thomas Graf <tgraf@...g.ch>
To:	roopa@...ulusnetworks.com
Cc:	jiri@...nulli.us, sfeldma@...il.com, jhs@...atatu.com,
	bcrl@...ck.org, john.fastabend@...il.com,
	stephen@...workplumber.org, linville@...driver.com,
	nhorman@...driver.com, nicolas.dichtel@...nd.com,
	vyasevic@...hat.com, f.fainelli@...il.com, buytenh@...tstofly.org,
	aviadr@...lanox.com, netdev@...r.kernel.org, davem@...emloft.net,
	shrijeet@...ulusnetworks.com, gospo@...ulusnetworks.com
Subject: Re: [RFC PATCH 4/4] bridge: make hw offload conditional on bridge
 and bridge port offload flags

On 11/21/14 at 02:49pm, roopa@...ulusnetworks.com wrote:
> +	    nla_put_u8(skb, IFLA_BRPORT_GUARD,
> +		           br_get_port_flag(p, IFLA_BRPORT_GUARD, BR_BPDU_GUARD)) ||

A helper taking nla_put_br_flag(port, skb, attrtype, brflag) would simplify
this code a lot.

> @@ -305,7 +312,9 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
>  
>  	br_set_state(p, state);
>  	br_log_state(p);
> -	netdev_sw_port_stp_update(p->dev, p->state);
> +
> +	if (BR_HW_OFFLOAD(p->br))
> +	    netdev_sw_port_set_attr(p->dev, IFLA_BRPORT_STATE, &p->state);

I assume the yet unfinished netdev_sw_port_set_attr() will call
netdev_sw_port_stp_update()?

> @@ -316,13 +325,34 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>  {
>  	if (tb[attrtype]) {
>  		u8 flag = nla_get_u8(tb[attrtype]);
> -		if (flag)
> -			p->flags |= mask;
> -		else
> +		if (flag) {
> +			flag_upper = flag & 0xf0
> +			if (!flag_upper || (flag_upper & BRPORT_KERNEL))
> +				p->flags |= mask;
> +			if ((flag_upper & BRPORT_HW_OFFLOAD) ||
> +				(BR_HW_OFFLOAD(p->br)))
> +				/* Also set the port flag in hw */
> +			netdev_sw_port_set_attr(p->dev, attrtype, 1);

I'm not sure I understand the || here. HW_OFFLOAD enabled on the
net_device is a conditional for all netdev_sw_port_set_attr() calls
and at the same time implies BRPORT_HW_OFFLOAD on all attributes.
As I read this code now, I don't see how you would offload individual
features to hardware.

> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 8f3f081..3ebd196 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -41,6 +41,8 @@
>  /* Path to usermode spanning tree program */
>  #define BR_STP_PROG	"/sbin/bridge-stp"
>  
> +#define BR_HW_OFFLOAD(br) !!(br->dev->features & NETIF_F_HW_OFFLOAD)

Let's not add more non type safe macros. A static inline seems like
a better fit here.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html