diff mbox

[dpdk-dev] igbvf: fix vlan filtering

Message ID 1446221643-10763-1-git-send-email-david.marchand@6wind.com (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

David Marchand Oct. 30, 2015, 4:14 p.m. UTC 
  From: Julien Meunier <julien.meunier@6wind.com>

Once posted through mailbox, we must check for nack from the PF and report an
error in this case.

Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 drivers/net/e1000/igb_ethdev.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Wenzhuo Lu Nov. 2, 2015, 7:21 a.m. UTC | #1 
Hi David, Julien,

> -----Original Message-----
> From: David Marchand [mailto:david.marchand@6wind.com]
> Sent: Saturday, October 31, 2015 12:14 AM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo; Julien Meunier
> Subject: [PATCH] igbvf: fix vlan filtering
> 
> From: Julien Meunier <julien.meunier@6wind.com>
> 
> Once posted through mailbox, we must check for nack from the PF and
> report an error in this case.
> 
> Signed-off-by: Julien Meunier <julien.meunier@6wind.com>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  drivers/net/e1000/igb_ethdev.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 3ab082e..84e593b 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -2494,6 +2494,7 @@ static int igbvf_set_vfta(struct e1000_hw *hw,
> uint16_t vid, bool on)  {
>  	struct e1000_mbx_info *mbx = &hw->mbx;
>  	uint32_t msgbuf[2];
> +	s32 err;
> 
>  	/* After set vlan, vlan strip will also be enabled in igb driver*/
>  	msgbuf[0] = E1000_VF_SET_VLAN;
> @@ -2502,7 +2503,14 @@ static int igbvf_set_vfta(struct e1000_hw *hw,
> uint16_t vid, bool on)
>  	if (on)
>  		msgbuf[0] |= E1000_VF_SET_VLAN_ADD;
> 
> -	return (mbx->ops.write_posted(hw, msgbuf, 2, 0));
> +	mbx->ops.write_posted(hw, msgbuf, 2, 0);
How about if write failed? I think we'd better check it.

> +	err = mbx->ops.read_posted(hw, msgbuf, 2, 0);
> +
> +	msgbuf[0] &= ~E1000_VT_MSGTYPE_CTS;
> +	if (!err && (msgbuf[0] == (E1000_VF_SET_VLAN |
> E1000_VT_MSGTYPE_NACK)))
> +		err = -EINVAL;
> +
> +	return err;
>  }
> 
>  static void igbvf_set_vfta_all(struct rte_eth_dev *dev, bool on)
> --
> 1.9.1
David Marchand Nov. 2, 2015, 4:32 p.m. UTC | #2 
Hello,

On Mon, Nov 2, 2015 at 8:21 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com> wrote:

>
> > @@ -2502,7 +2503,14 @@ static int igbvf_set_vfta(struct e1000_hw *hw,
> > uint16_t vid, bool on)
> >       if (on)
> >               msgbuf[0] |= E1000_VF_SET_VLAN_ADD;
> >
> > -     return (mbx->ops.write_posted(hw, msgbuf, 2, 0));
> > +     mbx->ops.write_posted(hw, msgbuf, 2, 0);
> How about if write failed? I think we'd better check it.
>

Yes, just trying to test this before resubmitting.

Thanks.
diff mbox

Patch

diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 3ab082e..84e593b 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2494,6 +2494,7 @@  static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on)
 {
 	struct e1000_mbx_info *mbx = &hw->mbx;
 	uint32_t msgbuf[2];
+	s32 err;
 
 	/* After set vlan, vlan strip will also be enabled in igb driver*/
 	msgbuf[0] = E1000_VF_SET_VLAN;
@@ -2502,7 +2503,14 @@  static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on)
 	if (on)
 		msgbuf[0] |= E1000_VF_SET_VLAN_ADD;
 
-	return (mbx->ops.write_posted(hw, msgbuf, 2, 0));
+	mbx->ops.write_posted(hw, msgbuf, 2, 0);
+	err = mbx->ops.read_posted(hw, msgbuf, 2, 0);
+
+	msgbuf[0] &= ~E1000_VT_MSGTYPE_CTS;
+	if (!err && (msgbuf[0] == (E1000_VF_SET_VLAN | E1000_VT_MSGTYPE_NACK)))
+		err = -EINVAL;
+
+	return err;
 }
 
 static void igbvf_set_vfta_all(struct rte_eth_dev *dev, bool on)