diff mbox series

[v2] net/qede: fix regression introduced by b10231aed1ed

Message ID f25988d5c0f726dd545c76bd74825c5b910dd0e8.1608732916.git.bnemeth@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Jerin Jacob
Headers
Series [v2] net/qede: fix regression introduced by b10231aed1ed |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS

Commit Message

Balazs Nemeth Dec. 23, 2020, 2:15 p.m. UTC 
  When calling rte_eth_promiscuous_enable(port_id) followed by
rte_eth_allmulticast_enable(port_id), the port is not in promisc mode
anymore. This patch ensures that promisc mode takes precedence over
allmulticast mode fixing the regression introduced by b10231aed1ed.

Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
---
 drivers/net/qede/qede_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)

--
2.26.2

Comments

Igor Russkikh Jan. 4, 2021, 12:44 p.m. UTC | #1 
> When calling rte_eth_promiscuous_enable(port_id) followed by
> rte_eth_allmulticast_enable(port_id), the port is not in promisc mode
> anymore. This patch ensures that promisc mode takes precedence over
> allmulticast mode fixing the regression introduced by b10231aed1ed.
> 
> Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
> ---
>  drivers/net/qede/qede_ethdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/qede/qede_ethdev.c
> b/drivers/net/qede/qede_ethdev.c
> index 549013557..3bec62d82 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -1885,6 +1885,8 @@ static int qede_allmulticast_enable(struct
> rte_eth_dev *eth_dev)
>  	    QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC;
>  	enum _ecore_status_t ecore_status;
> 
> +	if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
> +		type = QED_FILTER_RX_MODE_TYPE_PROMISC;
>  	ecore_status = qed_configure_filter_rx_mode(eth_dev, type);
> 
>  	return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN;

Hi Balazs, thanks for posting!

I think we need Fixes tag here.

Devendra, could you please check if thats enough? May be we should consider
more of internal states here? What if we'll do promisc_disable() after that?
Will allmulti state persist?

Igor
Rasesh Mody Jan. 4, 2021, 2:17 p.m. UTC | #2 
> From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> Sent: Monday, January 4, 2021 6:15 PM
> 
> > When calling rte_eth_promiscuous_enable(port_id) followed by
> > rte_eth_allmulticast_enable(port_id), the port is not in promisc mode
> > anymore. This patch ensures that promisc mode takes precedence over
> > allmulticast mode fixing the regression introduced by b10231aed1ed.
> >
> > Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
> > ---
> >  drivers/net/qede/qede_ethdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/qede/qede_ethdev.c
> > b/drivers/net/qede/qede_ethdev.c index 549013557..3bec62d82 100644
> > --- a/drivers/net/qede/qede_ethdev.c
> > +++ b/drivers/net/qede/qede_ethdev.c
> > @@ -1885,6 +1885,8 @@ static int qede_allmulticast_enable(struct
> > rte_eth_dev *eth_dev)
> >  	    QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC;
> >  	enum _ecore_status_t ecore_status;
> >
> > +	if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
> > +		type = QED_FILTER_RX_MODE_TYPE_PROMISC;
> >  	ecore_status = qed_configure_filter_rx_mode(eth_dev, type);
> >
> >  	return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN;
> 
> Hi Balazs, thanks for posting!
> 
> I think we need Fixes tag here.

Please Cc: stable@dpdk.org so that all the stable releases also integrate this change.

Thanks,
Rasesh

> 
> Devendra, could you please check if thats enough? May be we should
> consider more of internal states here? What if we'll do promisc_disable()
> after that?
> Will allmulti state persist?
> 
> Igor
Jerin Jacob Jan. 12, 2021, 4:50 a.m. UTC | #3 
On Mon, Jan 4, 2021 at 7:47 PM Rasesh Mody <rmody@marvell.com> wrote:
>
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> > Sent: Monday, January 4, 2021 6:15 PM
> >
> > > When calling rte_eth_promiscuous_enable(port_id) followed by
> > > rte_eth_allmulticast_enable(port_id), the port is not in promisc mode
> > > anymore. This patch ensures that promisc mode takes precedence over
> > > allmulticast mode fixing the regression introduced by b10231aed1ed.

Please add Fixes:

> > >
> > > Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>

Rasesh, Devendra,

Waiting for your ack to merge this.


> > > ---
> > >  drivers/net/qede/qede_ethdev.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/qede/qede_ethdev.c
> > > b/drivers/net/qede/qede_ethdev.c index 549013557..3bec62d82 100644
> > > --- a/drivers/net/qede/qede_ethdev.c
> > > +++ b/drivers/net/qede/qede_ethdev.c
> > > @@ -1885,6 +1885,8 @@ static int qede_allmulticast_enable(struct
> > > rte_eth_dev *eth_dev)
> > >         QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC;
> > >     enum _ecore_status_t ecore_status;
> > >
> > > +   if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
> > > +           type = QED_FILTER_RX_MODE_TYPE_PROMISC;
> > >     ecore_status = qed_configure_filter_rx_mode(eth_dev, type);
> > >
> > >     return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN;
> >
> > Hi Balazs, thanks for posting!
> >
> > I think we need Fixes tag here.
>
> Please Cc: stable@dpdk.org so that all the stable releases also integrate this change.
>
> Thanks,
> Rasesh
>
> >
> > Devendra, could you please check if thats enough? May be we should
> > consider more of internal states here? What if we'll do promisc_disable()
> > after that?
> > Will allmulti state persist?
> >
> > Igor
Rasesh Mody Jan. 12, 2021, 5:55 a.m. UTC | #4 
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, January 12, 2021 10:21 AM
> 
> On Mon, Jan 4, 2021 at 7:47 PM Rasesh Mody <rmody@marvell.com> wrote:
> >
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> > > Sent: Monday, January 4, 2021 6:15 PM
> > >
> > > > When calling rte_eth_promiscuous_enable(port_id) followed by
> > > > rte_eth_allmulticast_enable(port_id), the port is not in promisc
> > > > mode anymore. This patch ensures that promisc mode takes
> > > > precedence over allmulticast mode fixing the regression introduced by
> b10231aed1ed.
> 
> Please add Fixes:
> 
> > > >
> > > > Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
> 
> Rasesh, Devendra,
> 
> Waiting for your ack to merge this.
>
> 
> > > > ---
> > > >  drivers/net/qede/qede_ethdev.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/net/qede/qede_ethdev.c
> > > > b/drivers/net/qede/qede_ethdev.c index 549013557..3bec62d82
> 100644
> > > > --- a/drivers/net/qede/qede_ethdev.c
> > > > +++ b/drivers/net/qede/qede_ethdev.c
> > > > @@ -1885,6 +1885,8 @@ static int qede_allmulticast_enable(struct
> > > > rte_eth_dev *eth_dev)
> > > >         QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC;
> > > >     enum _ecore_status_t ecore_status;
> > > >
> > > > +   if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
> > > > +           type = QED_FILTER_RX_MODE_TYPE_PROMISC;
> > > >     ecore_status = qed_configure_filter_rx_mode(eth_dev, type);
> > > >
> > > >     return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN;
> > >
> > > Hi Balazs, thanks for posting!
> > >
> > > I think we need Fixes tag here.
> >
> > Please Cc: stable@dpdk.org so that all the stable releases also integrate this
> change.
> >
> > Thanks,
> > Rasesh
> >
> > >
> > > Devendra, could you please check if thats enough? May be we should
> > > consider more of internal states here? What if we'll do
> > > promisc_disable() after that?
> > > Will allmulti state persist?
> > >

Even if promiscuous mode is disabled after the change, allmulti mode setting will persist.
Fix looks good to me.

Acked-by: Rasesh Mody <rmody@marvell.com>

Thanks,
Rasesh

> > > Igor
Jerin Jacob Jan. 14, 2021, 1:29 p.m. UTC | #5 
On Tue, Jan 12, 2021 at 11:25 AM Rasesh Mody <rmody@marvell.com> wrote:
>
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Tuesday, January 12, 2021 10:21 AM
> >
> > On Mon, Jan 4, 2021 at 7:47 PM Rasesh Mody <rmody@marvell.com> wrote:
> > >
> > > > From: dev <dev-bounces@dpdk.org> On Behalf Of Igor Russkikh
> > > > Sent: Monday, January 4, 2021 6:15 PM
> > > >
> > > > > When calling rte_eth_promiscuous_enable(port_id) followed by
> > > > > rte_eth_allmulticast_enable(port_id), the port is not in promisc
> > > > > mode anymore. This patch ensures that promisc mode takes
> > > > > precedence over allmulticast mode fixing the regression introduced by
> > b10231aed1ed.
> >
> > Please add Fixes:
> >
> > > > >
> > > > > Signed-off-by: Balazs Nemeth <bnemeth@redhat.com>
> >

>
> Even if promiscuous mode is disabled after the change, allmulti mode setting will persist.
> Fix looks good to me.
>
> Acked-by: Rasesh Mody <rmody@marvell.com>


Fixed check-git-log.sh issues and applied to dpdk-next-net-mrvl/for-main. Thanks


>
> Thanks,
> Rasesh
>
> > > > Igor
>
diff mbox series

Patch

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 549013557..3bec62d82 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1885,6 +1885,8 @@  static int qede_allmulticast_enable(struct rte_eth_dev *eth_dev)
 	    QED_FILTER_RX_MODE_TYPE_MULTI_PROMISC;
 	enum _ecore_status_t ecore_status;

+	if (rte_eth_promiscuous_get(eth_dev->data->port_id) == 1)
+		type = QED_FILTER_RX_MODE_TYPE_PROMISC;
 	ecore_status = qed_configure_filter_rx_mode(eth_dev, type);

 	return ecore_status >= ECORE_SUCCESS ? 0 : -EAGAIN;