Re: [PATCH v2 net-next] net: hsr: Disable promiscuous mode in offload mode

[Vladimir Oltean]

On Mon, Jun 19, 2023 at 05:37:16PM +0530, Ravi Gunasekaran wrote:
> > It's not clear to me why you want to disable promiscuous mode. I'm not
> > familiar with HSR, but I assume you want the hardware to forward all the
> > packets between the two ports and not only specific DMACs.
> >
> > How does the underlying device implement "promiscuous mode" that you
> > benefit from disabling it?
>
> While creating an HSR interface using two slave nodes, the promiscuous mode
> is set via dev_set_promiscuity() in hsr_portdev_setup() for both the ports.
> And then in the HSR driver, a packet is forwarded to the other
> slave port (physical port) and also the HSR master if it is intended for it.
>
> Before forwarding, a check is done in
>
> static void hsr_forward_do(struct hsr_frame_info *frame)
> {
> ...
>
> if (hsr->proto_ops->drop_frame &&
>     hsr->proto_ops->drop_frame(frame, port))
>          continue;
>
> ...
> }
>
> And the drop_frame callback is as below
>
> bool hsr_drop_frame(struct hsr_frame_info *frame, struct hsr_port *port)
> {
>         if (port->dev->features & NETIF_F_HW_HSR_FWD)
>                 return prp_drop_frame(frame, port);
>
>         return false;
> }
>
>
> The driver drops these packets and does not forward to any port at all.
> But since promiscuous mode is enabled, CPU cycles are consumed. So benefit
> of disabling promiscuous mode is saving CPU cycles.
>
> So in this patch, I check for NETIF_F_HW_HSR_FWD and then take a
> call to enable/disable the promiscuous mode during HSR interface creation.

Can the hardware be configured to not send to the CPU packets that the
CPU is going to drop anyway? IFF_PROMISC is about receiving packets with
any MAC DA, not about sending all packets to the CPU. With offloading
drivers, there is a difference between the 2, because the RX path of a
port is not necessarily the same as the CPU receive path - the
destination of a packet can simply be another port.