netdev - Re: [PATCH] net: phy: b53: switchdev driver for Broadcom BCM53xx switches

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: <CACna6rwBUqebNpf9XC21+LxZtR7xLn6wM-KWuxHR=E0KT6MSdQ@mail.gmail.com>
Date:	Tue, 24 Feb 2015 23:50:14 +0100
From:	Rafał Miłecki <zajec5@...il.com>
To:	Andy Gospodarek <gospo@...ulusnetworks.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	Network Development <netdev@...r.kernel.org>,
	Jonas Gorski <jogo@...nwrt.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Hauke Mehrtens <hauke@...ke-m.de>,
	Felix Fietkau <nbd@...nwrt.org>, Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH] net: phy: b53: switchdev driver for Broadcom BCM53xx switches

On 24 February 2015 at 23:30, Andy Gospodarek <gospo@...ulusnetworks.com> wrote:
> On Tue, Feb 24, 2015 at 06:42:07PM +0100, Rafał Miłecki wrote:
>> BCM53xx is series of Broadcom Ethernet switches that can be found in
>> various (mostly home) routers.
>> They are quite simple switches with mainly just support for:
>> 1) Tagging incoming packets (PVID)
>> 2) Untagging outgoing packets
>> 3) Forwarding all packets across a single VLAN
>>
>> This driver is split into common code (module) and bus specific code.
>> Right now only PHY (MDIO) support is included, other could follow after
>> accepting this driver. It was successfully tested on BCM4706 SoC with
>> BCM53125.
>>
>> You could notice it's yet another try of submitting b53 driver. This
>> time it was modified to use recently introduced switchdev API which
>> hopefully make it possible to accept it mainline.
>
> I must confess I do not know the entire history of this driver, but I
> have a few comments....

I guess previous discussions were mostly focusing on API (like
swconfig), so a good review of b53 is still highly wanted!


>> diff --git a/drivers/net/phy/b53/b53_common.c b/drivers/net/phy/b53/b53_common.c
>> new file mode 100644
>> index 0000000..fce6b71
>> --- /dev/null
>> +++ b/drivers/net/phy/b53/b53_common.c
> [...]
>> +
>> +/*****************
>> + * Net device ops
>> + *****************/
>> +
>> +static netdev_tx_t b53_port_xmit(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +     dev_kfree_skb(skb);
>> +     dev->stats.tx_dropped++;
>> +
>> +     return NETDEV_TX_OK;
>> +}
>> +
>> (...)
>> +
>> +static const struct net_device_ops b53_port_netdev_ops = {
>> +     .ndo_start_xmit                 = b53_port_xmit,
>> +     .ndo_vlan_rx_add_vid            = b53_port_vlan_rx_add_vid,
>> +     .ndo_vlan_rx_kill_vid           = b53_port_vlan_rx_kill_vid,
>> +     .ndo_switch_parent_id_get       = b53_port_switch_parent_id_get,
>> +     .ndo_bridge_setlink             = b53_port_bridge_setlink,
>> +};
>
> I see there is an xmit function, but no napi registration at all.
>
> Is this simply because the SoC ethernet dev is doing all the frame rx?
> Is is able to demux the frames correctly, so it is clear which swXpY
> received the frame?  That will be necessary to ensure that standard
> applications that want to open sockets directly on the needed interfaces
> can still function properly.

If you take a look at b53_port_xmit you'll start understanding why
there is no NAPI ;)

As said in the commit message, these switches are really simple
devices. We can't actually send packets to the particular ports
(unless something has changed in the more recent hardware). We also
don't receive any packets from the switch itself! Really, switch
doesn't give as any IRQ when receiving frame, it doesn't put it in any
memory (DMA) we could access.
So we can't even consider knowing which port has received the frame.

So if we want to get any packets from switch port N we have to:
1) Assign some PVID to the port N
2) Put port N and "CPU port" in the same VLAN (matching PVID)
3) Starting with above switch will tag every packet received on port N
and forward it to the "CPU port"
4) Listen on the Ethernet device connected to the "CPU port" for
packets with a proper VLAN ID

Now with the above example there are 2 most common scenarios you may
want to consider:

1) Having full control over every port
It requires assigning unique VLAN for every single port. Any switching
(forwarding packets) has to be done in software then.

2) Having few groups of ports
This is the most common case for home routers. You usually want to
have group of "LAN" ports when you don't really care which port
provided a given packet. This also allows using switch for offloading
traffic between machines the same ports group.

So all this driver really does it allowing us to *configure* the
switch. It doesn't handle any RX/TX.

-- 
Rafał
--
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