On Fri, 8 Mar 2019 13:01:33 +0100
Daniel Kiper <daniel.ki...@oracle.com> wrote:
> On Thu, Mar 07, 2019 at 03:14:14PM +0000, Andre Przywara wrote:
> > From: Andrei Borzenkov <arvidj...@gmail.com>
> >
> > In respone to a BOOTREQUEST packet a BOOTP server would answer with a
> > BOOTREPLY packet, which ends the conversation for good.
> > DHCP uses a 4-way handshake, where the initial server respone is an OFFER,
> > which has to be answered with REQUEST by the client again, only to be
> > completed by an ACKNOWLEDGE packet from the server.
> >
> > Teach the grub_net_process_dhcp() function to deal with OFFER packets,
> > and treat ACK packets the same es BOOTREPLY packets.
> >
> > Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
> > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
>
> This patch has changed and you retained my RB and not explained why it
> has changed. Could you tell us what has happened? Next time please drop
> RB if you change the code significantly.
Mmh, weird. I didn't touch this patch at all, that's why just added your
R-B to the commit message. I think what happened is that the subtle change
in patch 05/10 (just removing the not needed brackets, as per your comment
on v2 4/9), caused the *diff* to be vastly different. I remember fixing it
up in the "git rebase -i" process. I just did a:
$ diff -u <(git show dhcp-v2~2:grub-core/net/bootp.c) <(git show
dhcp-v3~2:grub-core/net/bootp.c)
and that showed only unrelated changes.
So the resulting code change is actually identical, it's just that diff
took a different approach at expressing that.
Sorry for the confusion, hope that your R-b: still stands.
Cheers,
Andre.
> Hmmm... It seems to me that at least partially this is due to change in
> patch #5.
>
> Daniel
>
> > ---
> > grub-core/net/bootp.c | 78 +++++++++++++++++++++++++++++++++++--------
> > include/grub/net.h | 5 +++
> > 2 files changed, 70 insertions(+), 13 deletions(-)
> >
> > diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> > index bfde6ef10..d0482a7d5 100644
> > --- a/grub-core/net/bootp.c
> > +++ b/grub-core/net/bootp.c
> > @@ -30,6 +30,21 @@ enum
> > GRUB_DHCP_OPT_OVERLOAD_FILE = 1,
> > GRUB_DHCP_OPT_OVERLOAD_SNAME = 2,
> > };
> > +enum
> > +{
> > + GRUB_DHCP_MESSAGE_UNKNOWN,
> > + GRUB_DHCP_MESSAGE_DISCOVER,
> > + GRUB_DHCP_MESSAGE_OFFER,
> > + GRUB_DHCP_MESSAGE_REQUEST,
> > + GRUB_DHCP_MESSAGE_DECLINE,
> > + GRUB_DHCP_MESSAGE_ACK,
> > + GRUB_DHCP_MESSAGE_NAK,
> > + GRUB_DHCP_MESSAGE_RELEASE,
> > + GRUB_DHCP_MESSAGE_INFORM,
> > +};
> > +
> > +/* Max timeout when waiting for BOOTP/DHCP reply */
> > +#define GRUB_DHCP_MAX_PACKET_TIMEOUT 32
> >
> > #define GRUB_BOOTP_MAX_OPTIONS_SIZE 64
> >
> > @@ -443,22 +458,59 @@ grub_net_process_dhcp (struct grub_net_buff *nb,
> > struct grub_net_card *card = iface->card;
> > const struct grub_net_bootp_packet *bp = (const struct
> > grub_net_bootp_packet *) nb->data;
> > grub_size_t size = nb->tail - nb->data;
> > + const grub_uint8_t *opt;
> > + grub_uint8_t opt_len, type;
> > + grub_uint32_t srv_id = 0;
> > +
> > + opt = find_dhcp_option (bp, size, GRUB_NET_DHCP_MESSAGE_TYPE, &opt_len);
> > + if (opt && opt_len == 1)
> > + type = *opt;
> > + else
> > + type = GRUB_DHCP_MESSAGE_UNKNOWN;
> > +
> > + opt = find_dhcp_option (bp, size, GRUB_NET_DHCP_SERVER_IDENTIFIER,
> > &opt_len);
> > + if (opt && opt_len == sizeof (srv_id))
> > + srv_id = grub_get_unaligned32 (opt);
> >
> > - name = grub_xasprintf ("%s:dhcp", card->name);
> > - if (!name)
> > + /*
> > + * If we received BOOTP reply or DHCPACK, proceed with configuration.
> > + * Otherwise store offered address and server id for later processing
> > + * of DHCPACK.
> > + * xid and srv_id are stored in network order so do not need conversion.
> > + */
> > + if ((!iface->srv_id && type == GRUB_DHCP_MESSAGE_UNKNOWN)
> > + || (iface->srv_id && type == GRUB_DHCP_MESSAGE_ACK
> > + && bp->ident == iface->xid
> > + && srv_id == iface->srv_id))
> > {
> > - grub_print_error ();
> > - return;
> > + name = grub_xasprintf ("%s:dhcp", card->name);
> > + if (!name)
> > + {
> > + grub_print_error ();
> > + return;
> > + }
> > + grub_net_configure_by_dhcp_ack (name, card, 0, bp, size, 0, 0, 0);
> > + grub_free (name);
> > + if (grub_errno)
> > + grub_print_error ();
> > + else
> > + grub_net_network_level_interface_unregister (iface);
> > + }
> > + else if (!iface->srv_id && type == GRUB_DHCP_MESSAGE_OFFER && srv_id)
> > + {
> > + iface->srv_id = srv_id;
> > + iface->my_ip = bp->your_ip;
> > + /* Reset retransmission timer */
> > + iface->dhcp_tmo = iface->dhcp_tmo_left = 1;
> > + }
> > + else if (iface->srv_id && type == GRUB_DHCP_MESSAGE_NAK
> > + && bp->ident == iface->xid
> > + && srv_id == iface->srv_id)
> > + {
> > + iface->xid = iface->srv_id = iface->my_ip = 0;
> > + /* Reset retransmission timer */
> > + iface->dhcp_tmo = iface->dhcp_tmo_left = 1;
> > }
> > - grub_net_configure_by_dhcp_ack (name, card, 0, bp, size, 0, 0, 0);
> > - grub_free (name);
> > - if (grub_errno)
> > - grub_print_error ();
> > - else
> > - if (grub_memcmp (iface->name, card->name, grub_strlen (card->name)) ==
> > 0 &&
> > - grub_memcmp (iface->name + grub_strlen (card->name),
> > - ":dhcp_tmp", sizeof (":dhcp_tmp") - 1) == 0)
> > - grub_net_network_level_interface_unregister (iface);
> > }
> >
> > static char
> > diff --git a/include/grub/net.h b/include/grub/net.h
> > index f7b546446..68a9f1311 100644
> > --- a/include/grub/net.h
> > +++ b/include/grub/net.h
> > @@ -292,6 +292,9 @@ struct grub_net_network_level_interface
> > struct grub_net_bootp_packet *dhcp_ack;
> > grub_size_t dhcp_acklen;
> > grub_uint16_t vlantag;
> > + grub_uint32_t xid; /* DHCPv4 transaction id */
> > + grub_uint32_t srv_id; /* DHCPv4 server_identifier */
> > + grub_uint32_t my_ip; /* DHCPv4 offered IP address */
> > unsigned dhcp_tmo_left; /* DHCPv4 running retransmission timeout */
> > unsigned dhcp_tmo; /* DHCPv4 current retransmission timeout */
> > void *data;
> > @@ -460,6 +463,8 @@ enum
> > GRUB_NET_BOOTP_ROOT_PATH = 0x11,
> > GRUB_NET_BOOTP_EXTENSIONS_PATH = 0x12,
> > GRUB_NET_DHCP_OVERLOAD = 52,
> > + GRUB_NET_DHCP_MESSAGE_TYPE = 53,
> > + GRUB_NET_DHCP_SERVER_IDENTIFIER = 54,
> > GRUB_NET_DHCP_TFTP_SERVER_NAME = 66,
> > GRUB_NET_DHCP_BOOTFILE_NAME = 67,
> > GRUB_NET_BOOTP_END = 0xff
> > --
> > 2.17.1
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
