> +static bool tpmr_should_relay_reserved(const u8 *dst)
> +{
> + static const u8 prefix[5] = { 0x01, 0x80, 0xc2, 0x00, 0x00 };
> +
> + if (memcmp(dst, prefix, sizeof(prefix)) != 0)
> + return true;
> +
> + return dst[5] != 0x01;
I would assume the traffic is mostly unicast, with only a few
multicast thrown in. So would it not make sense to first test for
multicast? Only if it is multicast do you need to look deeper into
the address?
Also, is_multicast_ether_addr() exists.
> + eh = eth_hdr(skb);
> + if (is_multicast_ether_addr(eh->h_dest) &&
> + !tpmr_should_relay_reserved(eh->h_dest))
> + return RX_HANDLER_PASS;
Ah, you already have it here. I still think this can be optimized.
> + /* Enforce MTU consistency between the two members. */
> + if (tpmr->n_ports == 1) {
> + struct net_device *first;
> +
> + first = rtnl_dereference(tpmr->ports[tpmr->ports[0].slot].dev);
> + if (first && first->mtu != slave_dev->mtu) {
> + NL_SET_ERR_MSG(extack,
> + "Member MTU must match the other
> member's");
> + return -EINVAL;
> + }
> + }
This seems to assume that tpmr->ports[0] is filled first. But what
about:
ip link eth0 set master tpmr0
ip link eth1 set master tpmr0
ip link eth0 set nomaster
ip link eth42 set master tpmr0
> +static int tpmr_del_slave(struct net_device *dev, struct net_device
> *slave_dev)
> +{
> + struct tpmr_priv *tpmr = netdev_priv(dev);
> + struct tpmr_port *port = NULL;
> + unsigned int slot;
> +
> + ASSERT_RTNL();
> +
> + for (slot = 0; slot < TPMR_MAX_PORTS; slot++) {
> + if (rtnl_dereference(tpmr->ports[slot].dev) == slave_dev) {
> + port = &tpmr->ports[slot];
> + break;
> + }
> + }
For the above to work, there needs to be some juggling here.
> +static int tpmr_device_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *slave_dev = netdev_notifier_info_to_dev(ptr);
> + struct net_device *master;
> + struct tpmr_priv *tpmr;
> +
> + if (slave_dev->reg_state == NETREG_REGISTERED) {
> + master = netdev_master_upper_dev_get(slave_dev);
> + if (!master || master->rtnl_link_ops != &tpmr_link_ops)
> + return NOTIFY_DONE;
> + } else {
> + return NOTIFY_DONE;
> + }
> +
> + tpmr = netdev_priv(master);
> +
> + switch (event) {
> + case NETDEV_CHANGE:
> + case NETDEV_UP:
> + case NETDEV_DOWN:
> + tpmr_update_carrier(tpmr);
> + break;
> + case NETDEV_UNREGISTER:
> + tpmr_del_slave(master, slave_dev);
> + break;
> + }
> + return NOTIFY_DONE;
Maybe you also want to act on NETDEV_PRECHANGEMTU and reject it?
> +static void tpmr_get_drvinfo(struct net_device *dev,
> + struct ethtool_drvinfo *drvinfo)
> +{
> + strscpy(drvinfo->driver, TPMR_DRV_NAME, sizeof(drvinfo->driver));
> + strscpy(drvinfo->version, TPMR_DRV_VERSION, sizeof(drvinfo->version));
Driver version is meaningless because it never changes. Please leave
it empty, and the core will fill it with the kernel version.
I also wounder if this is the correct home, or if it should be in net,
next to bridge? I don't know...
Andrew
