Skip to content

Commit

Permalink
pixiecore: set Content-Length when serving the kernel/initrd.
Browse files Browse the repository at this point in the history 
iPXE appears to have *really* poor performance (orders of magnitude worse) if
it doesn't know the length of the kernel/initrds that it's downloading. Without
this change, booting CoreOS takes longer than I've had patience to wait. With
this change, the bottleneck becomes the network transfer speed.

Fixes #10.
  • Loading branch information
@danderson
danderson committed Sep 15, 2016
1 parent d040347 commit 3aa7695
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 34 deletions.
49 changes: 31 additions & 18 deletions pixiecore/booters.go
View file
Expand Up @@ -71,22 +71,31 @@ func (s *staticBooter) BootSpec(m Machine) (*Spec, error) {
return s.spec, nil
}

func (s *staticBooter) serveFile(path string) (io.ReadCloser, error) {
func (s *staticBooter) serveFile(path string) (io.ReadCloser, int64, error) {
if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") {
resp, err := http.Get(path)
if err != nil {
return nil, err
return nil, -1, err
}
if resp.StatusCode != http.StatusOK {
resp.Body.Close()
return nil, fmt.Errorf("%s: %s", path, http.StatusText(resp.StatusCode))
return nil, -1, fmt.Errorf("%s: %s", path, http.StatusText(resp.StatusCode))
}
return resp.Body, nil
return resp.Body, resp.ContentLength, nil
}
f, err := os.Open(path)
if err != nil {
return nil, -1, err
}
fi, err := f.Stat()
if err != nil {
f.Close()
return nil, -1, err
}
return os.Open(path)
return f, fi.Size(), nil
}

func (s *staticBooter) ReadBootFile(id ID) (io.ReadCloser, error) {
func (s *staticBooter) ReadBootFile(id ID) (io.ReadCloser, int64, error) {
path := string(id)
switch {
case path == "kernel":
Expand All @@ -95,19 +104,19 @@ func (s *staticBooter) ReadBootFile(id ID) (io.ReadCloser, error) {
case strings.HasPrefix(path, "initrd-"):
i, err := strconv.Atoi(string(path[7:]))
if err != nil || i < 0 || i >= len(s.initrd) {
return nil, fmt.Errorf("no file with ID %q", id)
return nil, -1, fmt.Errorf("no file with ID %q", id)
}
return s.serveFile(s.initrd[i])

case strings.HasPrefix(path, "other-"):
i, err := strconv.Atoi(string(path[6:]))
if err != nil || i < 0 || i >= len(s.otherIDs) {
return nil, fmt.Errorf("no file with ID %q", id)
return nil, -1, fmt.Errorf("no file with ID %q", id)
}
return s.serveFile(s.otherIDs[i])
}

return nil, fmt.Errorf("no file with ID %q", id)
return nil, -1, fmt.Errorf("no file with ID %q", id)
}

func (s *staticBooter) WriteBootFile(ID, io.Reader) error {
Expand Down Expand Up @@ -229,37 +238,41 @@ func (b *apibooter) BootSpec(m Machine) (*Spec, error) {
return &ret, nil
}

func (b *apibooter) ReadBootFile(id ID) (io.ReadCloser, error) {
func (b *apibooter) ReadBootFile(id ID) (io.ReadCloser, int64, error) {
urlStr, err := getURL(id, &b.key)
if err != nil {
return nil, err
return nil, -1, err
}

u, err := url.Parse(urlStr)
if err != nil {
return nil, fmt.Errorf("%q is not an URL", urlStr)
return nil, -1, fmt.Errorf("%q is not an URL", urlStr)
}
var ret io.ReadCloser
var (
ret io.ReadCloser
sz int64 = -1
)
if u.Scheme == "file" {
// TODO serveFile
ret, err = os.Open(u.Path)
} else {
// urlStr will get reparsed by http.Get, which is mildly
// wasteful, but the code looks nicer than constructing a
// Request.
resp, err := http.Get(urlStr)
if err != nil {
return nil, err
return nil, -1, err
}
if resp.StatusCode != 200 {
return nil, fmt.Errorf("GET %q failed: %s", urlStr, resp.Status)
return nil, -1, fmt.Errorf("GET %q failed: %s", urlStr, resp.Status)
}

ret, err = resp.Body, nil
ret, sz, err = resp.Body, resp.ContentLength, nil
}
if err != nil {
return nil, err
return nil, -1, err
}
return ret, nil
return ret, sz, nil
}

func (b *apibooter) WriteBootFile(id ID, body io.Reader) error {
Expand Down
5 changes: 4 additions & 1 deletion pixiecore/booters_test.go
View file
Expand Up @@ -43,7 +43,7 @@ func mustWrite(dir, path, contents string) {
}
}

func mustRead(f io.ReadCloser, err error) string {
func mustRead(f io.ReadCloser, sz int64, err error) string {
if err != nil {
panic(err)
}
Expand All @@ -52,6 +52,9 @@ func mustRead(f io.ReadCloser, err error) string {
if err != nil {
panic(err)
}
if sz >= 0 && int64(len(bs)) != sz {
panic(fmt.Errorf("sz = %d, but ReadCloser has %d bytes", sz, len(bs)))
}
return string(bs)
}

Expand Down
12 changes: 4 additions & 8 deletions pixiecore/cli/cli.go
View file
Expand Up @@ -18,7 +18,6 @@ package cli // import "go.universe.tf/netboot/pixiecore/cli"
import (
"fmt"
"io/ioutil"
"net"
"os"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -71,7 +70,7 @@ func todo(msg string, args ...interface{}) {
func serverConfigFlags(cmd *cobra.Command) {
cmd.Flags().BoolP("debug", "d", false, "Log more things that aren't directly related to booting a recognized client")
cmd.Flags().BoolP("log-timestamps", "t", false, "Add a timestamp to each log line")
cmd.Flags().IPP("listen-addr", "l", net.IPv4zero, "IPv4 address to listen on")
cmd.Flags().StringP("listen-addr", "l", "", "IPv4 address to listen on")
cmd.Flags().IntP("port", "p", 80, "Port to listen on for HTTP")
cmd.Flags().Bool("dhcp-no-bind", false, "Handle DHCP traffic without binding to the DHCP server port")
cmd.Flags().String("ipxe-bios", "", "Path to an iPXE binary for BIOS/UNDI")
Expand All @@ -97,7 +96,7 @@ func serverFromFlags(cmd *cobra.Command) *pixiecore.Server {
if err != nil {
fatalf("Error reading flag: %s", err)
}
addr, err := cmd.Flags().GetIP("listen-addr")
addr, err := cmd.Flags().GetString("listen-addr")
if err != nil {
fatalf("Error reading flag: %s", err)
}
Expand All @@ -122,9 +121,6 @@ func serverFromFlags(cmd *cobra.Command) *pixiecore.Server {
fatalf("Error reading flag: %s", err)
}

if addr != nil && addr.To4() == nil {
fatalf("Listen address must be IPv4")
}
if httpPort <= 0 {
fatalf("HTTP port must be >0")
}
Expand Down Expand Up @@ -154,8 +150,8 @@ func serverFromFlags(cmd *cobra.Command) *pixiecore.Server {
if debug {
ret.Debug = ret.Log
}
if addr != nil {
ret.Address = addr.String()
if addr != "" {
ret.Address = addr
}

return ret
Expand Down
5 changes: 4 additions & 1 deletion pixiecore/http.go
View file
Expand Up @@ -108,13 +108,16 @@ func (s *Server) handleFile(w http.ResponseWriter, r *http.Request) {
http.Error(w, "missing filename", http.StatusBadRequest)
}

f, err := s.Booter.ReadBootFile(ID(name))
f, sz, err := s.Booter.ReadBootFile(ID(name))
if err != nil {
s.log("HTTP", "Error getting file %q (query %q from %s): %s", name, r.URL, r.RemoteAddr, err)
http.Error(w, "couldn't get file", http.StatusInternalServerError)
return
}
defer f.Close()
if sz >= 0 {
w.Header().Set("Content-Length", strconv.FormatInt(sz, 10))
}
if _, err = io.Copy(w, f); err != nil {
s.log("HTTP", "Copy of %q to %s (query %q) failed: %s", name, r.RemoteAddr, r.URL, err)
return
Expand Down
13 changes: 8 additions & 5 deletions pixiecore/http_test.go
View file
Expand Up @@ -28,9 +28,11 @@ import (

type booterFunc func(Machine) (*Spec, error)

func (b booterFunc) BootSpec(m Machine) (*Spec, error) { return b(m) }
func (b booterFunc) ReadBootFile(id ID) (io.ReadCloser, error) { return nil, errors.New("no") }
func (b booterFunc) WriteBootFile(id ID, r io.Reader) error { return errors.New("no") }
func (b booterFunc) BootSpec(m Machine) (*Spec, error) { return b(m) }
func (b booterFunc) ReadBootFile(id ID) (io.ReadCloser, int64, error) {
return nil, -1, errors.New("no")
}
func (b booterFunc) WriteBootFile(id ID, r io.Reader) error { return errors.New("no") }

var logSync sync.Mutex

Expand Down Expand Up @@ -149,8 +151,9 @@ boot kernel initrd=initrd0 initrd=initrd1 thing=http://localhost:1234/_/file?nam
type readBootFile string

func (b readBootFile) BootSpec(m Machine) (*Spec, error) { return nil, nil }
func (b readBootFile) ReadBootFile(id ID) (io.ReadCloser, error) {
return ioutil.NopCloser(bytes.NewBuffer([]byte(fmt.Sprintf("%s %s", id, b)))), nil
func (b readBootFile) ReadBootFile(id ID) (io.ReadCloser, int64, error) {
d := fmt.Sprintf("%s %s", id, b)
return ioutil.NopCloser(bytes.NewBuffer([]byte(d))), int64(len(d)), nil
}
func (b readBootFile) WriteBootFile(id ID, r io.Reader) error { return errors.New("no") }

Expand Down
9 changes: 8 additions & 1 deletion pixiecore/pixiecore.go
View file
Expand Up @@ -102,7 +102,12 @@ type Booter interface {
// the client machine's request.
BootSpec(m Machine) (*Spec, error)
// Get the bytes corresponding to an ID given in Spec.
ReadBootFile(id ID) (io.ReadCloser, error)
//
// Additionally returns the total number of bytes in the
// ReadCloser, or -1 if the size is unknown. Be warned, returning
// -1 will make the boot process orders of magnitude slower due to
// poor ipxe behavior.
ReadBootFile(id ID) (io.ReadCloser, int64, error)
// Write the given Reader to an ID given in Spec.
WriteBootFile(id ID, body io.Reader) error
}
Expand Down Expand Up @@ -214,6 +219,8 @@ func (s *Server) Serve() error {
// blocking.
s.errs = make(chan error, 5)

s.debug("Init", "Starting Pixiecore goroutines")

go func() { s.errs <- s.serveDHCP(dhcp) }()
go func() { s.errs <- s.servePXE(pxe) }()
go func() { s.errs <- s.serveTFTP(tftp) }()
Expand Down

0 comments on commit 3aa7695

Please sign in to comment.