| Message ID | 1421967198-16667-1-git-send-email-linux@rasmusvillemoes.dk |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 2015-01-22 at 23:53 +0100, Rasmus Villemoes wrote: > The comment explains the intention, but vid has type u16. Before the > inner shift, it is promoted to int, which has plenty of space for all > vid's bits, so nothing is dropped. Use a simple mask instead. > > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks Rasmus, I will add your patch to my queue.
On 1/22/15, 2:53 PM, "Rasmus Villemoes" <linux@rasmusvillemoes.dk> wrote: >The comment explains the intention, but vid has type u16. Before the >inner shift, it is promoted to int, which has plenty of space for all >vid's bits, so nothing is dropped. Use a simple mask instead. > >Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> >--- > drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c >b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c >index 275423d4f777..b1c57d0166a9 100644 >--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c >+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c >@@ -335,7 +335,7 @@ static s32 fm10k_update_xc_addr_pf(struct fm10k_hw >*hw, u16 glort, > return FM10K_ERR_PARAM; > > /* drop upper 4 bits of VLAN ID */ >- vid = (vid << 4) >> 4; >+ vid &= 0x0fff; > > /* record fields */ > mac_update.mac_lower = cpu_to_le32(((u32)mac[2] << 24) | Good catch! I noticed this too and was getting a patch together to address this. The difference is that I was planning on not silently accepting an invalid VLAN ID to begin with and returning FM10K_ERR_PARAM if the VLAN was invalid, which I think is a better approach for this situation. If it's alright with you, I'll generate the patch shortly and credit you via your Reported-by. Cheers, Matthew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Jan 24 2015, "Vick, Matthew" <matthew.vick@intel.com> wrote: > Good catch! I noticed this too and was getting a patch together to address > this. > > The difference is that I was planning on not silently accepting an invalid > VLAN ID to begin with and returning FM10K_ERR_PARAM if the VLAN was > invalid, which I think is a better approach for this situation. If it's > alright with you, I'll generate the patch shortly and credit you via your > Reported-by. Sure, do what you think is best. Rasmus -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1/23/15, 4:50 PM, "Rasmus Villemoes" <linux@rasmusvillemoes.dk> wrote: >On Sat, Jan 24 2015, "Vick, Matthew" <matthew.vick@intel.com> wrote: > >> Good catch! I noticed this too and was getting a patch together to >>address >> this. >> >> The difference is that I was planning on not silently accepting an >>invalid >> VLAN ID to begin with and returning FM10K_ERR_PARAM if the VLAN was >> invalid, which I think is a better approach for this situation. If it's >> alright with you, I'll generate the patch shortly and credit you via >>your >> Reported-by. > >Sure, do what you think is best. > >Rasmus Sounds good. I'll get the patch created and validated through Jeff's tree. Cheers, Matthew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c index 275423d4f777..b1c57d0166a9 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c @@ -335,7 +335,7 @@ static s32 fm10k_update_xc_addr_pf(struct fm10k_hw *hw, u16 glort, return FM10K_ERR_PARAM; /* drop upper 4 bits of VLAN ID */ - vid = (vid << 4) >> 4; + vid &= 0x0fff; /* record fields */ mac_update.mac_lower = cpu_to_le32(((u32)mac[2] << 24) |
The comment explains the intention, but vid has type u16. Before the inner shift, it is promoted to int, which has plenty of space for all vid's bits, so nothing is dropped. Use a simple mask instead. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)