[SLOF] [PATCH 1/2] ping: use gateway address for routing

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> On 22.04.2016 07:23, Nikunj A Dadhania wrote:
>> Thomas Huth <thuth at redhat.com> writes:
>> 
>>> On 13.04.2016 09:03, Nikunj A Dadhania wrote:
>>>> ping was failing for machine across the subnet with
>>>> statically assinged IP address. The parsed gateway address
>>>> was ignored in the stack because the router variable was not
>>>> set.
>>>>
>>>> Currently, ping does not take a netmask parameter, set netmask parameter
>>>> with the common top order bits of client IP and gateway IP.
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>>>> ---
>>>>  clients/net-snk/app/netapps/ping.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/clients/net-snk/app/netapps/ping.c b/clients/net-snk/app/netapps/ping.c
>>>> index 2c7dadb..1522792 100644
>>>> --- a/clients/net-snk/app/netapps/ping.c
>>>> +++ b/clients/net-snk/app/netapps/ping.c
>>>> @@ -163,6 +163,12 @@ ping(int argc, char *argv[])
>>>>  
>>>>  	} else {
>>>>  		memcpy(&fn_ip.own_ip, &ping_args.client_ip.integer, 4);
>>>> +		if (ping_args.gateway_ip.integer) {
>>>> +			uint32_t netmask;
>>>> +			netmask = ping_args.gateway_ip.integer & ping_args.client_ip.integer;
>>>> +			set_ipv4_netmask(netmask & 0xFFFFFF00);
>>>
>>> That netmask calculation looks somewhat strange to me ... what's the
>>> rationale here?
>> 
>> Ping currently only supports IPv4. Case here was that subnet mask is not
>> provided and we are trying to guess the subnet mask. Dont think this
>> will be accurate always, but will work.
>
> I think this will be very fragile. For example, let's assume client
> address is 192.168.1.10, router address is 192.168.1.1 and TFTP server
> address is 192.168.3.2 (i.e. TFTP server is in another subnet).
>
> With your calculation, you get netmask = 192.168.1.0.

Oops, yes, i should have got it as 255.255.255.0

> When the
> send_ipv4() code then tries to find out whether the TFTP server is in
> the same network as the client (to find out whether a router MAC has to
> be used), it does:
>
>    if((subnet_mask & own_ip) == (subnet_mask & ip->ip_dst)) ...
>
> and thus in above example, it will think that the TFTP server is in the
> same subnet as the client and fail to use the router address.
>
>>> If you need automatic netmask calculation, shouldn't
>>> that rather be done by the type of IP address that has been specified?
>> 
>> Here its only IPv4. We still dont have ipv6 support here. I have that in
>> my list though.
>
> I didn't mean IPv6, I was talking about IPv4 address classes (sorry,
> should have used that term). For Class A, B and C, the default netmask
> should be applied as long as no CIDR prefix length has been specified
> (see the first table on
> https://en.wikipedia.org/wiki/IPv4_subnetting_reference)
>
>>> (or maybe via an "/xx" suffix of the specified IP address string?)
>> 
>> I am not sure about this, can you please elaborate?
>
> Sorry again for not using the right terms ... I meant the specifying the
> prefix length (and thus the netmask) with a CIDR prefix notation, see
> https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_notation
>
> That means, it might maybe be useful to support specifying the client
> address with a prefix length like this:
>
>  load net:192.168.3.2,filename,192.168.1.10/24
>
> What do you think?

So that will be client-ip/nn, we need to take care we dont break old
cases. how about

Currently, its this

[bootp,ipv6,dhcp,]siaddr,filename,ciaddr,giaddr,bootp-retries,tftp-retries

Change that to:

[bootp,ipv6,dhcp,]siaddr,filename,ciaddr,giaddr,bootp-retries,tftp-retries,netmask


One more argument at the end.

Regards
Nikunj