Re: [PATCH] net: take care of bonding in build_skb_flow_key

[Wengang Wang <wen.gang.wang@oracle.com>]

在 2016年01月06日 15:35, zhuyj 写道:
> IMHO, "The path MTU is set to the active slave device, not the bonding 
> master."
> Can we set PMTU to bonding master when path MTU is set to the active 
> slave device?
>
Actually the route is set on bonding master, not on any slave, the 
trying to set PMTU to the active slave failed(it didn't found a route on 
slave).
So "set PMTU to bonding master when path MTU is set to the active slave 
device" is lacking a base.

thanks,
wengang

> If not appropriate, you can ignore it.
>
> Best Regards!
> Zhu Yanjun
>
> On 01/06/2016 03:06 PM, Wengang Wang wrote:
>> 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 place to do that.
>> Let's go through the simplified follow:
>>
>> ...
>> 1) Fragmentation.
>>    --This is is done is against the bonding master device(device MTU 
>> and path MTU)
>> 2) bond_start_xmit
>> 3) ipoib_start_xmit(slaves are IPoIB interfaces)
>>
>> For 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 
>> internal limit 2044, drops the packet and try to update PMTU.
>>     without the patch, it tried update PMTU on slave device(no 
>> changes to master).
>>
>> the seconds send comes, since no changes happen on bonding 
>> master(PMTU), the fragment size is still 7000 and the behavior is 
>> just the same as the first send.
>>
>> With the patch, the bonding master PMTU is changed to 2044 after the 
>> first send(hopefully), for the seconds send the fragment size is set 
>> to 2044.
>>
>> To fix in bonding code, I don't find where we can.
>>
>> thanks,
>> wengang
>>
>> 在 2016年01月06日 14:19, zhuyj 写道:
>>>
>>> IMHO, this should fix in bonding driver because the active slave mtu 
>>> should be the same with the master.
>>> bonding master's mtu is changed to path MTU, then slave dev's MTU 
>>> 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 
>>>> device and
>>>> deal with path MTU there: The path MTU is set to the active slave 
>>>> device, not
>>>> the bonding master.
>>>>
>>>> The patch tries to fix the issue by letting build_skb_flow_key() 
>>>> take care
>>>> of the transition of device index from bonding slave to the master.
>>>>
>>>> Signed-off-by: Wengang Wang <wen.gang.wang@oracle.com>
>>>> ---
>>>>   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 
>>>> *fl4, const struct sk_buff *skb,
>>>>                      const struct sock *sk)
>>>>   {
>>>>       const struct iphdr *iph = ip_hdr(skb);
>>>> -    int oif = skb->dev->ifindex;
>>>> +    int oif;
>>>> +    struct net_device *master = NULL;
>>>> +
>>>>       u8 tos = RT_TOS(iph->tos);
>>>>       u8 prot = iph->protocol;
>>>>       u32 mark = skb->mark;
>>>>   +    if (skb->dev->flags & IFF_SLAVE)
>>>> +        master = netdev_master_upper_dev_get(skb->dev);
>>>> +    if (master)
>>>> +        oif = master->ifindex;
>>>> +    else
>>>> +        oif = skb->dev->ifindex;
>>>> +
>>>>       __build_flow_key(fl4, sk, iph, oif, tos, prot, mark, 0);
>>>>   }
>>>
>>
>