Allocate 6 bytes to copy MAC addresses when unmarshalling ethernet match fields

[rojkov]

Currently we are trying to unmarshal data to an uninitialized struct whose net.HardwareAddr fields are set to nil. In this case the copy() function silently does nothing.

Initialize the fields to slices of bytes of length 6 before copying data to them.

Signed-off-by: Dmitry Rozhkov [email protected]

[wenyingd]

Hi @rojkov thanks for working on this patch. I have one comment on this patch:

OpenFlow 1.5 is the major version we planned to support for a long time ( OpenFlow1.3 is deprecated ), so please also change the code in path openflow15/match.go if necessary.

[vanytsvetkov]

Hello, @wenyingd @tnqn @antoninbas !
Please give a motion on this problem, the solution of which was proposed by Mr. Rozhkov.
Below is an example of a really simple mistake that cannot be fixed for almost two years.

package main

import (
	"encoding/hex"
	"fmt"

	"antrea.io/libOpenflow/openflow15"
)

// Random OpenFlow1.5 Match from my OVS bridge;
var MatchWithMac, _ = hex.DecodeString("00010037800000040000000580000606deadbeef010280000806deadbeef030480000a020800800014010680001a02000580001c02000500")

func PrintEthFields(bytes []byte) (err error) {
	ofMatch := openflow15.NewMatch()

	err = ofMatch.UnmarshalBinary(bytes)
	if err != nil {
		return err
	}

	for _, field := range ofMatch.Fields {
		switch f := field.Value.(type) {
		case *openflow15.EthSrcField:
			fmt.Printf("Ethernet src field: %s\n", f.EthSrc)
			if f.EthSrc == nil {
				return fmt.Errorf("ethernet src field is nil")
			}
		case *openflow15.EthDstField:
			fmt.Printf("Ethernet dst field: %s\n", f.EthDst)
			if f.EthDst == nil {
				return fmt.Errorf("ethernet dst field is nil")
			}
		}
	}

	return nil
}

func main() {
	err := PrintEthFields(MatchWithMac)
	if err != nil {
		panic(err)
	}
}

Correct output is:

Ethernet dst field: de:ad:be:ef:01:02
Ethernet src field: de:ad:be:ef:03:04

[antoninbas]

@vanytsvetkov We are fine with the patch, but the original comment from @wenyingd was not addressed.

The change should be applied to openflow15/match.go as well. I would say a unit test would be nice as well.

Feel free to take over this patch since we haven't heard back from @rojkov

[rojkov]

@vanytsvetkov please feel free to resubmit the patch under your credentials. I work on completely unrelated projects nowadays.

[tnqn]

Closing the PR as #62 has been merged. Thanks @rojkov @vanytsvetkov for fixing it.