add -D (don't fragment) to ping6

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

add -D (don't fragment) to ping6

David Gwynne-5
this brings -D over to ping6.

ok?

Index: ping.8
===================================================================
RCS file: /cvs/src/sbin/ping/ping.8,v
retrieving revision 1.60
diff -u -p -r1.60 ping.8
--- ping.8 10 Nov 2018 05:03:23 -0000 1.60
+++ ping.8 10 Nov 2018 05:25:30 -0000
@@ -79,7 +79,7 @@
 .Op Fl w Ar maxwait
 .Ar host
 .Nm ping6
-.Op Fl dEefHLmnqv
+.Op Fl DdEefHLmnqv
 .Op Fl c Ar count
 .Op Fl h Ar hoplimit
 .Op Fl I Ar sourceaddr
@@ -116,10 +116,10 @@ If
 .Ar count
 is 0, send an unlimited number of packets.
 .It Fl D
-.Pq IPv4 only
+Don't fragment.
 Set the
 .Dv Don't Fragment
-bit.
+bit on IPv4 frames, or disable fragmentation of transmitted IPv6 packets.
 .It Fl d
 Set the
 .Dv SO_DEBUG
Index: ping.c
===================================================================
RCS file: /cvs/src/sbin/ping/ping.c,v
retrieving revision 1.231
diff -u -p -r1.231 ping.c
--- ping.c 10 Nov 2018 05:03:23 -0000 1.231
+++ ping.c 10 Nov 2018 05:25:31 -0000
@@ -291,7 +291,7 @@ main(int argc, char *argv[])
  preload = 0;
  datap = &outpack[ECHOLEN + ECHOTMLEN];
  while ((ch = getopt(argc, argv, v6flag ?
-    "c:dEefHh:I:i:Ll:mNnp:qS:s:T:V:vw:" :
+    "c:DdEefHh:I:i:Ll:mNnp:qS:s:T:V:vw:" :
     "DEI:LRS:c:defHi:l:np:qs:T:t:V:vw:")) != -1) {
  switch(ch) {
  case 'c':
@@ -681,6 +681,13 @@ main(int argc, char *argv[])
  warn("setsockopt(IPV6_TVAL)"); /* XXX err? */
  }
 
+ if (df) {
+ optval = 1;
+ if (setsockopt(s, IPPROTO_IPV6, IPV6_DONTFRAG,
+    &optval, (socklen_t)sizeof(optval)) < 0)
+ warn("setsockopt(IPV6_DONTFRAG"); /* err? */
+ }
+
  optval = 1;
  if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVPKTINFO, &optval,
     (socklen_t)sizeof(optval)) < 0)
@@ -2166,7 +2173,7 @@ usage(void)
 {
  if (v6flag) {
  fprintf(stderr,
-    "usage: ping6 [-dEefHLmnqv] [-c count] [-h hoplimit] "
+    "usage: ping6 [-DdEefHLmnqv] [-c count] [-h hoplimit] "
     "[-I sourceaddr]\n\t[-i wait] [-l preload] [-p pattern] "
     "[-s packetsize] [-T toskeyword]\n\t"
     "[-V rtable] [-w maxwait] host\n");

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

Jason McIntyre-2
On Sat, Nov 10, 2018 at 03:27:19PM +1000, David Gwynne wrote:
> this brings -D over to ping6.
>
> ok?
>

morning.

could we reduce the entire text to just "Disable fragmentation of
transmitted packets.", without special casing ip4/6?

or perhaps "Don;t fragment transmitted packets.", since that is maybe a
term people will search for?

jmc

> Index: ping.8
> ===================================================================
> RCS file: /cvs/src/sbin/ping/ping.8,v
> retrieving revision 1.60
> diff -u -p -r1.60 ping.8
> --- ping.8 10 Nov 2018 05:03:23 -0000 1.60
> +++ ping.8 10 Nov 2018 05:25:30 -0000
> @@ -79,7 +79,7 @@
>  .Op Fl w Ar maxwait
>  .Ar host
>  .Nm ping6
> -.Op Fl dEefHLmnqv
> +.Op Fl DdEefHLmnqv
>  .Op Fl c Ar count
>  .Op Fl h Ar hoplimit
>  .Op Fl I Ar sourceaddr
> @@ -116,10 +116,10 @@ If
>  .Ar count
>  is 0, send an unlimited number of packets.
>  .It Fl D
> -.Pq IPv4 only
> +Don't fragment.
>  Set the
>  .Dv Don't Fragment
> -bit.
> +bit on IPv4 frames, or disable fragmentation of transmitted IPv6 packets.
>  .It Fl d
>  Set the
>  .Dv SO_DEBUG
> Index: ping.c
> ===================================================================
> RCS file: /cvs/src/sbin/ping/ping.c,v
> retrieving revision 1.231
> diff -u -p -r1.231 ping.c
> --- ping.c 10 Nov 2018 05:03:23 -0000 1.231
> +++ ping.c 10 Nov 2018 05:25:31 -0000
> @@ -291,7 +291,7 @@ main(int argc, char *argv[])
>   preload = 0;
>   datap = &outpack[ECHOLEN + ECHOTMLEN];
>   while ((ch = getopt(argc, argv, v6flag ?
> -    "c:dEefHh:I:i:Ll:mNnp:qS:s:T:V:vw:" :
> +    "c:DdEefHh:I:i:Ll:mNnp:qS:s:T:V:vw:" :
>      "DEI:LRS:c:defHi:l:np:qs:T:t:V:vw:")) != -1) {
>   switch(ch) {
>   case 'c':
> @@ -681,6 +681,13 @@ main(int argc, char *argv[])
>   warn("setsockopt(IPV6_TVAL)"); /* XXX err? */
>   }
>  
> + if (df) {
> + optval = 1;
> + if (setsockopt(s, IPPROTO_IPV6, IPV6_DONTFRAG,
> +    &optval, (socklen_t)sizeof(optval)) < 0)
> + warn("setsockopt(IPV6_DONTFRAG"); /* err? */
> + }
> +
>   optval = 1;
>   if (setsockopt(s, IPPROTO_IPV6, IPV6_RECVPKTINFO, &optval,
>      (socklen_t)sizeof(optval)) < 0)
> @@ -2166,7 +2173,7 @@ usage(void)
>  {
>   if (v6flag) {
>   fprintf(stderr,
> -    "usage: ping6 [-dEefHLmnqv] [-c count] [-h hoplimit] "
> +    "usage: ping6 [-DdEefHLmnqv] [-c count] [-h hoplimit] "
>      "[-I sourceaddr]\n\t[-i wait] [-l preload] [-p pattern] "
>      "[-s packetsize] [-T toskeyword]\n\t"
>      "[-V rtable] [-w maxwait] host\n");
>

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

Klemens Nanni-2
In reply to this post by David Gwynne-5
Yes, although I agree with jmc on simpler wording. FWIW, FreeBSD just
goes with "-D Set the Don't Fragment bit" as well.

On Sat, Nov 10, 2018 at 03:27:19PM +1000, David Gwynne wrote:
> @@ -681,6 +681,13 @@ main(int argc, char *argv[])
>   warn("setsockopt(IPV6_TVAL)"); /* XXX err? */
>   }
>  
> + if (df) {
> + optval = 1;
> + if (setsockopt(s, IPPROTO_IPV6, IPV6_DONTFRAG,
> +    &optval, (socklen_t)sizeof(optval)) < 0)
> + warn("setsockopt(IPV6_DONTFRAG"); /* err? */
I'm inclined to error out on all setsockopt(2) calls.  Crafting special
packets is always deliberate, thus the code should fail if not all
requested options could be set.  Users may simply retry (with other
options) to get it working instead of keep sending different packets.

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

Theo de Raadt-2
Klemens Nanni <[hidden email]> wrote:

> Yes, although I agree with jmc on simpler wording. FWIW, FreeBSD just
> goes with "-D Set the Don't Fragment bit" as well.

Suppose so.

> On Sat, Nov 10, 2018 at 03:27:19PM +1000, David Gwynne wrote:
> > @@ -681,6 +681,13 @@ main(int argc, char *argv[])
> >   warn("setsockopt(IPV6_TVAL)"); /* XXX err? */
> >   }
> >  
> > + if (df) {
> > + optval = 1;
> > + if (setsockopt(s, IPPROTO_IPV6, IPV6_DONTFRAG,
> > +    &optval, (socklen_t)sizeof(optval)) < 0)
> > + warn("setsockopt(IPV6_DONTFRAG"); /* err? */
> I'm inclined to error out on all setsockopt(2) calls.  Crafting special
> packets is always deliberate, thus the code should fail if not all
> requested options could be set.  Users may simply retry (with other
> options) to get it working instead of keep sending different packets.

I agree on that point.

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

Alexander Bluhm
In reply to this post by Klemens Nanni-2
On Sat, Nov 10, 2018 at 12:54:49PM +0100, Klemens Nanni wrote:
> Yes, although I agree with jmc on simpler wording. FWIW, FreeBSD just
> goes with "-D Set the Don't Fragment bit" as well.

There is no "Don't Fragment bit" in IPv6, so this is wrong.

In IPv4 we do set the don't fragment bit in the packet to prevent
router fragmentation, in IPv6 we prevent fragmentation in the local
output path.

The "Don;t fragment transmitted packets." is not precise.  In IPv6
it would be the other way around "Don;t transmit fragmented packets."

What about "-D Prevent IP fragments."?

bluhm

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

Jason McIntyre-2
On Sat, Nov 10, 2018 at 10:50:14AM -0700, Alexander Bluhm wrote:

> On Sat, Nov 10, 2018 at 12:54:49PM +0100, Klemens Nanni wrote:
> > Yes, although I agree with jmc on simpler wording. FWIW, FreeBSD just
> > goes with "-D Set the Don't Fragment bit" as well.
>
> There is no "Don't Fragment bit" in IPv6, so this is wrong.
>
> In IPv4 we do set the don't fragment bit in the packet to prevent
> router fragmentation, in IPv6 we prevent fragmentation in the local
> output path.
>
> The "Don;t fragment transmitted packets." is not precise.  In IPv6
> it would be the other way around "Don;t transmit fragmented packets."
>
> What about "-D Prevent IP fragments."?
>
> bluhm
>

that sounds fine too. should we talk about packets though? "Don't
fragment IP packets." or "Prevent fragmentation of IP packets." or
somesuch?

jmc

Reply | Threaded
Open this post in threaded view
|

Re: add -D (don't fragment) to ping6

David Gwynne-5


> On 11 Nov 2018, at 4:13 am, Jason McIntyre <[hidden email]> wrote:
>
> On Sat, Nov 10, 2018 at 10:50:14AM -0700, Alexander Bluhm wrote:
>> On Sat, Nov 10, 2018 at 12:54:49PM +0100, Klemens Nanni wrote:
>>> Yes, although I agree with jmc on simpler wording. FWIW, FreeBSD just
>>> goes with "-D Set the Don't Fragment bit" as well.
>>
>> There is no "Don't Fragment bit" in IPv6, so this is wrong.

It could be argued that since

>>
>> In IPv4 we do set the don't fragment bit in the packet to prevent
>> router fragmentation, in IPv6 we prevent fragmentation in the local
>> output path.
>>
>> The "Don;t fragment transmitted packets." is not precise.  In IPv6
>> it would be the other way around "Don;t transmit fragmented packets."
>>
>> What about "-D Prevent IP fragments."?
>>
>> bluhm
>>
>
> that sounds fine too. should we talk about packets though? "Don't
> fragment IP packets." or "Prevent fragmentation of IP packets." or
> somesuch?

I think it's important to have "Don't Fragment" in there in some form.

I'll commit it with "Don't fragment IP packets".

dlg