All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
@ 2019-01-12 10:28 wangyunjian
  2019-01-12 14:54 ` Andrew Lunn
  0 siblings, 1 reply; 6+ messages in thread
From: wangyunjian @ 2019-01-12 10:28 UTC (permalink / raw)
  To: netdev; +Cc: xudingke, Yunjian Wang

From: Yunjian Wang <wangyunjian@huawei.com>

The skb header should be set to ethernet header before using
is_skb_forwardable(including dev->hard_header_len length).

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 net/bridge/br_forward.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 5372e20..74b688b 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -36,10 +36,10 @@ static inline int should_deliver(const struct net_bridge_port *p,
 
 int br_dev_queue_push_xmit(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
+	skb_push(skb, ETH_HLEN);
 	if (!is_skb_forwardable(skb->dev, skb))
 		goto drop;
 
-	skb_push(skb, ETH_HLEN);
 	br_drop_fake_rtable(skb);
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL &&
@@ -97,10 +97,10 @@ static void __br_forward(const struct net_bridge_port *to,
 		net = dev_net(indev);
 	} else {
 		if (unlikely(netpoll_tx_running(to->br->dev))) {
+			skb_push(skb, ETH_HLEN);
 			if (!is_skb_forwardable(skb->dev, skb)) {
 				kfree_skb(skb);
 			} else {
-				skb_push(skb, ETH_HLEN);
 				br_netpoll_send_skb(to, skb);
 			}
 			return;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
  2019-01-12 10:28 [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable wangyunjian
@ 2019-01-12 14:54 ` Andrew Lunn
  2019-01-14  4:05   ` wangyunjian
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-01-12 14:54 UTC (permalink / raw)
  To: wangyunjian; +Cc: netdev, xudingke

On Sat, Jan 12, 2019 at 06:28:27PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> The skb header should be set to ethernet header before using
> is_skb_forwardable(including dev->hard_header_len length).

Hi Yunjian

I don't see any recent changes in this code. Do you know why this is
needed now, yet it seemed to work before? Do you have a fixes: tag for
where it broke?

Thanks
	Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
  2019-01-12 14:54 ` Andrew Lunn
@ 2019-01-14  4:05   ` wangyunjian
  2019-01-14 13:51     ` Andrew Lunn
  0 siblings, 1 reply; 6+ messages in thread
From: wangyunjian @ 2019-01-14  4:05 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, xudingke

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Saturday, January 12, 2019 10:55 PM
> To: wangyunjian <wangyunjian@huawei.com>
> Cc: netdev@vger.kernel.org; xudingke <xudingke@huawei.com>
> Subject: Re: [PATCH net] bridge: Fix ethernet header pointer before check
> skb forwardable
> 
> On Sat, Jan 12, 2019 at 06:28:27PM +0800, wangyunjian wrote:
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > The skb header should be set to ethernet header before using
> > is_skb_forwardable(including dev->hard_header_len length).
> 
> Hi Yunjian
> 
> I don't see any recent changes in this code. Do you know why this is needed
> now, yet it seemed to work before? Do you have a fixes: tag for where it
> broke?
> 
> Thanks
> 	Andrew

Hi Andrew

This change is commit f6367b4660dde412f9b7af94763efb1d89cefb74(
bridge: use is_skb_forwardable in forward path).

I found it when I was testing the vNIC(virtio-net) 's mtu. 
I add 2 port on linux bridge br using following commands
brctl addbr br
brctl addif br eth0
brctl addif br eth1

The mtu of eth0 and eth1 is 1500,so the expect result is packet larger than
1500 cannot pass through eth0 and eth1.But currently, when I send a 1504
packet from eth0 to eth1, the packet passes through success, it means eth1's
mtu limit donen't take effect.

Thanks
	Yunjian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
  2019-01-14  4:05   ` wangyunjian
@ 2019-01-14 13:51     ` Andrew Lunn
  2019-01-14 14:04       ` Nikolay Aleksandrov
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2019-01-14 13:51 UTC (permalink / raw)
  To: wangyunjian; +Cc: netdev, xudingke

On Mon, Jan 14, 2019 at 04:05:23AM +0000, wangyunjian wrote:
> > -----Original Message-----
> > From: Andrew Lunn [mailto:andrew@lunn.ch]
> > Sent: Saturday, January 12, 2019 10:55 PM
> > To: wangyunjian <wangyunjian@huawei.com>
> > Cc: netdev@vger.kernel.org; xudingke <xudingke@huawei.com>
> > Subject: Re: [PATCH net] bridge: Fix ethernet header pointer before check
> > skb forwardable
> > 
> > On Sat, Jan 12, 2019 at 06:28:27PM +0800, wangyunjian wrote:
> > > From: Yunjian Wang <wangyunjian@huawei.com>
> > >
> > > The skb header should be set to ethernet header before using
> > > is_skb_forwardable(including dev->hard_header_len length).
> > 
> > Hi Yunjian
> > 
> > I don't see any recent changes in this code. Do you know why this is needed
> > now, yet it seemed to work before? Do you have a fixes: tag for where it
> > broke?
> > 
> > Thanks
> > 	Andrew
> 
> Hi Andrew
> 
> This change is commit f6367b4660dde412f9b7af94763efb1d89cefb74(
> bridge: use is_skb_forwardable in forward path).
> 
> I found it when I was testing the vNIC(virtio-net) 's mtu. 
> I add 2 port on linux bridge br using following commands
> brctl addbr br
> brctl addif br eth0
> brctl addif br eth1
> 
> The mtu of eth0 and eth1 is 1500,so the expect result is packet larger than
> 1500 cannot pass through eth0 and eth1.But currently, when I send a 1504
> packet from eth0 to eth1, the packet passes through success, it means eth1's
> mtu limit donen't take effect.

Hi Yunjian

Thanks for the explanation. Please could you improve the description
in the patch. Also, add a Fixes tag:

Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward path")

Thanks
	Andrew       

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
  2019-01-14 13:51     ` Andrew Lunn
@ 2019-01-14 14:04       ` Nikolay Aleksandrov
  2019-01-15  3:08         ` wangyunjian
  0 siblings, 1 reply; 6+ messages in thread
From: Nikolay Aleksandrov @ 2019-01-14 14:04 UTC (permalink / raw)
  To: wangyunjian; +Cc: Andrew Lunn, netdev, xudingke

On 14/01/2019 15:51, Andrew Lunn wrote:
> On Mon, Jan 14, 2019 at 04:05:23AM +0000, wangyunjian wrote:
>>> -----Original Message-----
>>> From: Andrew Lunn [mailto:andrew@lunn.ch]
>>> Sent: Saturday, January 12, 2019 10:55 PM
>>> To: wangyunjian <wangyunjian@huawei.com>
>>> Cc: netdev@vger.kernel.org; xudingke <xudingke@huawei.com>
>>> Subject: Re: [PATCH net] bridge: Fix ethernet header pointer before check
>>> skb forwardable
>>>
>>> On Sat, Jan 12, 2019 at 06:28:27PM +0800, wangyunjian wrote:
>>>> From: Yunjian Wang <wangyunjian@huawei.com>
>>>>
>>>> The skb header should be set to ethernet header before using
>>>> is_skb_forwardable(including dev->hard_header_len length).
>>>
>>> Hi Yunjian
>>>
>>> I don't see any recent changes in this code. Do you know why this is needed
>>> now, yet it seemed to work before? Do you have a fixes: tag for where it
>>> broke?
>>>
>>> Thanks
>>> 	Andrew
>>
>> Hi Andrew
>>
>> This change is commit f6367b4660dde412f9b7af94763efb1d89cefb74(
>> bridge: use is_skb_forwardable in forward path).
>>
>> I found it when I was testing the vNIC(virtio-net) 's mtu. 
>> I add 2 port on linux bridge br using following commands
>> brctl addbr br
>> brctl addif br eth0
>> brctl addif br eth1
>>
>> The mtu of eth0 and eth1 is 1500,so the expect result is packet larger than
>> 1500 cannot pass through eth0 and eth1.But currently, when I send a 1504
>> packet from eth0 to eth1, the packet passes through success, it means eth1's
>> mtu limit donen't take effect.
> 
> Hi Yunjian
> 
> Thanks for the explanation. Please could you improve the description
> in the patch. Also, add a Fixes tag:
> 
> Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward path")
> 
> Thanks
> 	Andrew       
> 

I just noticed this patch thanks to the discussion, please add the
bridge maintainers to the CC list next time as well.

Thanks,
 Nik

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable
  2019-01-14 14:04       ` Nikolay Aleksandrov
@ 2019-01-15  3:08         ` wangyunjian
  0 siblings, 0 replies; 6+ messages in thread
From: wangyunjian @ 2019-01-15  3:08 UTC (permalink / raw)
  To: Nikolay Aleksandrov, Andrew Lunn; +Cc: netdev, xudingke



> -----Original Message-----
> From: Nikolay Aleksandrov [mailto:nikolay@cumulusnetworks.com]
> Sent: Monday, January 14, 2019 10:05 PM
> To: wangyunjian <wangyunjian@huawei.com>
> Cc: Andrew Lunn <andrew@lunn.ch>; netdev@vger.kernel.org; xudingke
> <xudingke@huawei.com>
> Subject: Re: [PATCH net] bridge: Fix ethernet header pointer before check
> skb forwardable
> 
> On 14/01/2019 15:51, Andrew Lunn wrote:
> > On Mon, Jan 14, 2019 at 04:05:23AM +0000, wangyunjian wrote:
> >>> -----Original Message-----
> >>> From: Andrew Lunn [mailto:andrew@lunn.ch]
> >>> Sent: Saturday, January 12, 2019 10:55 PM
> >>> To: wangyunjian <wangyunjian@huawei.com>
> >>> Cc: netdev@vger.kernel.org; xudingke <xudingke@huawei.com>
> >>> Subject: Re: [PATCH net] bridge: Fix ethernet header pointer before
> >>> check skb forwardable
> >>>
> >>> On Sat, Jan 12, 2019 at 06:28:27PM +0800, wangyunjian wrote:
> >>>> From: Yunjian Wang <wangyunjian@huawei.com>
> >>>>
> >>>> The skb header should be set to ethernet header before using
> >>>> is_skb_forwardable(including dev->hard_header_len length).
> >>>
> >>> Hi Yunjian
> >>>
> >>> I don't see any recent changes in this code. Do you know why this is
> >>> needed now, yet it seemed to work before? Do you have a fixes: tag
> >>> for where it broke?
> >>>
> >>> Thanks
> >>> 	Andrew
> >>
> >> Hi Andrew
> >>
> >> This change is commit f6367b4660dde412f9b7af94763efb1d89cefb74(
> >> bridge: use is_skb_forwardable in forward path).
> >>
> >> I found it when I was testing the vNIC(virtio-net) 's mtu.
> >> I add 2 port on linux bridge br using following commands brctl addbr
> >> br brctl addif br eth0 brctl addif br eth1
> >>
> >> The mtu of eth0 and eth1 is 1500,so the expect result is packet
> >> larger than
> >> 1500 cannot pass through eth0 and eth1.But currently, when I send a
> >> 1504 packet from eth0 to eth1, the packet passes through success, it
> >> means eth1's mtu limit donen't take effect.
> >
> > Hi Yunjian
> >
> > Thanks for the explanation. Please could you improve the description
> > in the patch. Also, add a Fixes tag:
> >
> > Fixes: f6367b4660dd ("bridge: use is_skb_forwardable in forward path")
> >
> > Thanks
> > 	Andrew
> >
> 
> I just noticed this patch thanks to the discussion, please add the bridge
> maintainers to the CC list next time as well.
> 
> Thanks,
>  Nik

OK, I got your point, will send the v2 later.

Thanks,
	Yunjian


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-15  3:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-12 10:28 [PATCH net] bridge: Fix ethernet header pointer before check skb forwardable wangyunjian
2019-01-12 14:54 ` Andrew Lunn
2019-01-14  4:05   ` wangyunjian
2019-01-14 13:51     ` Andrew Lunn
2019-01-14 14:04       ` Nikolay Aleksandrov
2019-01-15  3:08         ` wangyunjian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.