From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wengang Wang Subject: Re: [PATCH] net: take care of bonding in build_skb_flow_key Date: Wed, 6 Jan 2016 15:06:18 +0800 Message-ID: <568CBCEA.20702@oracle.com> References: <1452059368-7527-1-git-send-email-wen.gang.wang@oracle.com> <568CB1FD.4030907@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE To: zhuyj , netdev@vger.kernel.org Return-path: Received: from aserp1040.oracle.com ([141.146.126.69]:47057 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751140AbcAFHCx (ORCPT ); Wed, 6 Jan 2016 02:02:53 -0500 In-Reply-To: <568CB1FD.4030907@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi Yanjun, Thanks for your review. Master MTU is same as that for slaves. Maybe fixing in bonding driver is a good idea, but I don't find a good=20 place to do that. Let's go through the simplified follow: =2E.. 1) Fragmentation. --This is is done is against the bonding master device(device MTU=20 and path MTU) 2) bond_start_xmit 3) ipoib_start_xmit(slaves are IPoIB interfaces) =46or the first send 1) fragment size is 7000(in my case) 2) bond_start_xmit its self is fine 3) ipoib_start_xmit sees the packet size 7000 is larger than the=20 internal limit 2044, drops the packet and try to update PMTU. without the patch, it tried update PMTU on slave device(no changes= =20 to master). the seconds send comes, since no changes happen on bonding master(PMTU)= ,=20 the fragment size is still 7000 and the behavior is just the same as th= e=20 first send. With the patch, the bonding master PMTU is changed to 2044 after the=20 first send(hopefully), for the seconds send the fragment size is set to= =20 2044. To fix in bonding code, I don't find where we can. thanks, wengang =E5=9C=A8 2016=E5=B9=B401=E6=9C=8806=E6=97=A5 14:19, zhuyj =E5=86=99=E9= =81=93: > > IMHO, this should fix in bonding driver because the active slave mtu=20 > should be the same with the master. > bonding master's mtu is changed to path MTU, then slave dev's MTU=20 > should be changed, too. > > Zhu Yanjun > On 01/06/2016 01:49 PM, Wengang Wang wrote: >> A problem is found that we are looking for route basing a bonding=20 >> device and >> deal with path MTU there: The path MTU is set to the active slave=20 >> device, not >> the bonding master. >> >> The patch tries to fix the issue by letting build_skb_flow_key() tak= e=20 >> care >> of the transition of device index from bonding slave to the master. >> >> Signed-off-by: Wengang Wang >> --- >> net/ipv4/route.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/net/ipv4/route.c b/net/ipv4/route.c >> index 85f184e..3053f10 100644 >> --- a/net/ipv4/route.c >> +++ b/net/ipv4/route.c >> @@ -523,11 +523,20 @@ static void build_skb_flow_key(struct flowi4=20 >> *fl4, const struct sk_buff *skb, >> const struct sock *sk) >> { >> const struct iphdr *iph =3D ip_hdr(skb); >> - int oif =3D skb->dev->ifindex; >> + int oif; >> + struct net_device *master =3D NULL; >> + >> u8 tos =3D RT_TOS(iph->tos); >> u8 prot =3D iph->protocol; >> u32 mark =3D skb->mark; >> + if (skb->dev->flags & IFF_SLAVE) >> + master =3D netdev_master_upper_dev_get(skb->dev); >> + if (master) >> + oif =3D master->ifindex; >> + else >> + oif =3D skb->dev->ifindex; >> + >> __build_flow_key(fl4, sk, iph, oif, tos, prot, mark, 0); >> } >