Daniel Alvarez Sanchez <dalva...@redhat.com> writes:

> Thanks Lucas, this makes sense. There is something that this patch is
> fixing and I'm not sure why.
> Maybe someone can shed some light:
>
> Using datapath from OVS master, and a setup where we have a physical
> interface connected to
> an OVS bridge (br-ex) connected to another OVS bridge (br-int) through a
> patch port, there's a lot
> of retransmissions of TCP packets when connecting from the host to a VM
> connected to br-int.
> The retransmissions seem to be due to a wrong checksum from the VM to the
> host and only after
> a few attempts, the checksum is correct and the host sends the ACK back.
> The packets I am
> sending using netcat are very small so there shouldn't be a problem with
> the MTU. However, could
> it be a side effect of this patch that the checksum gets now correctly
> received at the host?
>
> As a side not: if instead from connecting to the VM from the host I do it
> from a namespace where
> I have an OVS internal port connected to br-ex, then I don't see the
> checksum problems.

I'm concerned - I don't see why this checksum issue would occur.
Additionally, I see no reason for this patch to clear it up.

Do you have a clear reproducer?

> Acked-by: Daniel Alvarez <dalva...@redhat.com>
> Tested-by: Daniel Alvarez <dalva...@redhat.com>
>
> On Thu, May 17, 2018 at 1:27 PM, <lucasago...@gmail.com> wrote:
>
>> From: Lucas Alvares Gomes <lucasago...@gmail.com>
>>
>> The commit [0] partially fixed the problem but in RHEL 7.5 neither
>> .{min, max}_mtu or 'ndo_change_mtu' were being set/implemented for
>> vport-internal_dev.c.
>>
>> As pointed out by commit [0], the ndo_change_mtu function pointer has been
>> moved from 'struct net_device_ops' to 'struct net_device_ops_extended'
>> on RHEL 7.5.
>>
>> So this patch fixes the backport issue by setting the
>> .extended.ndo_change_mtu when necessary.
>>
>> [0] 39ca338374abe367e28a2247bac9159695f19710
>> ---
>>  datapath/vport-internal_dev.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c
>> index 3cb8d06b2..16f4aaeee 100644
>> --- a/datapath/vport-internal_dev.c
>> +++ b/datapath/vport-internal_dev.c
>> @@ -88,7 +88,7 @@ static const struct ethtool_ops internal_dev_ethtool_ops
>> = {
>>         .get_link       = ethtool_op_get_link,
>>  };
>>
>> -#if    !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) &&
>> !defined(HAVE_RHEL7_MAX_MTU)
>> +#ifndef HAVE_NET_DEVICE_WITH_MAX_MTU
>>  static int internal_dev_change_mtu(struct net_device *dev, int new_mtu)
>>  {
>>         if (new_mtu < ETH_MIN_MTU) {
>> @@ -155,6 +155,8 @@ static const struct net_device_ops
>> internal_dev_netdev_ops = {
>>         .ndo_set_mac_address = eth_mac_addr,
>>  #if    !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) &&
>> !defined(HAVE_RHEL7_MAX_MTU)
>>         .ndo_change_mtu = internal_dev_change_mtu,
>> +#elif  !defined(HAVE_NET_DEVICE_WITH_MAX_MTU) &&
>> defined(HAVE_RHEL7_MAX_MTU)
>> +       .extended.ndo_change_mtu = internal_dev_change_mtu,
>>  #endif
>>         .ndo_get_stats64 = (void *)internal_get_stats,
>>  };
>> --
>> 2.17.0
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to