Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Jun 2023 15:19:55 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Ravi Gunasekaran <r-gunasekaran@ti.com>
Cc:     Ido Schimmel <idosch@nvidia.com>, kuba@kernel.org,
        Nikolay Aleksandrov <razor@blackwall.org>, davem@davemloft.net,
        edumazet@google.com, pabeni@redhat.com, bigeasy@linutronix.de,
        simon.horman@corigine.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, rogerq@kernel.org
Subject: Re: [PATCH v2 net-next] net: hsr: Disable promiscuous mode in
 offload mode
Message-ID: <20230627121955.4mhwwkc3i4knrsxg@skbuf>
References: <20230614114710.31400-1-r-gunasekaran@ti.com>
 <20230615223736.0577fb11@kernel.org>
 <ZJA4fIH6vm9cO2VG@shredder>
 <a93ff762-215b-fbc1-9e23-b186421cb176@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a93ff762-215b-fbc1-9e23-b186421cb176@ti.com>
Precedence: bulk

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.