Skip to content

Commit

Permalink
Improve handling of string IEs (#322)
Browse files Browse the repository at this point in the history
* Use the copy built-in to copy string bytes to buffer, instead of
  copying bytes one-by-one with a for loop. This is more efficient
  (confirmed with new Benchmark functions) and more correct (in case
  the string includes UTF-8 characters which use more than 1 byte).
* Increase max string length from 65,534 to 65,535 (as per the
  IPFIX RFC 7011).

Signed-off-by: Antonin Bas <abas@vmware.com>
  • Loading branch information
antoninbas committed Sep 29, 2023
1 parent d0294af commit b8729db
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
28 changes: 13 additions & 15 deletions pkg/entities/ie.go
Original file line number Diff line number Diff line change
Expand Up @@ -485,16 +485,14 @@ func EncodeToIEDataType(dataType IEDataType, val interface{}) ([]byte, error) {
if len(v) < 255 {
encodedBytes = make([]byte, len(v)+1)
encodedBytes[0] = uint8(len(v))
for i, b := range v {
encodedBytes[i+1] = byte(b)
}
} else if len(v) < 65535 {
copy(encodedBytes[1:], v)
} else if len(v) <= math.MaxUint16 {
encodedBytes = make([]byte, len(v)+3)
encodedBytes[0] = byte(255)
binary.BigEndian.PutUint16(encodedBytes[1:3], uint16(len(v)))
for i, b := range v {
encodedBytes[i+3] = byte(b)
}
copy(encodedBytes[3:], v)
} else {
return nil, fmt.Errorf("provided String value is too long and cannot be encoded: len=%d, maxlen=%d", len(v), math.MaxUint16)
}
return encodedBytes, nil
}
Expand Down Expand Up @@ -560,15 +558,15 @@ func encodeInfoElementValueToBuff(element InfoElementWithValue, buffer []byte, i
v := element.GetStringValue()
if len(v) < 255 {
buffer[index] = uint8(len(v))
for i, b := range v {
buffer[i+index+1] = byte(b)
}
} else if len(v) < 65535 {
buffer[index] = byte(255)
// See https://pkg.go.dev/builtin#copy
// As a special case, it also will copy bytes from a string to a slice of bytes.
copy(buffer[index+1:], v)
} else if len(v) <= math.MaxUint16 {
buffer[index] = byte(255) // marker byte for long strings
binary.BigEndian.PutUint16(buffer[index+1:index+3], uint16(len(v)))
for i, b := range v {
buffer[i+index+3] = byte(b)
}
copy(buffer[index+3:], v)
} else {
return fmt.Errorf("provided String value is too long and cannot be encoded: len=%d, maxlen=%d", len(v), math.MaxUint16)
}
default:
return fmt.Errorf("API supports only valid information elements with datatypes given in RFC7011")
Expand Down
36 changes: 36 additions & 0 deletions pkg/entities/ie_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package entities

import (
"net"
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var macAddress, _ = net.ParseMAC("aa:bb:cc:dd:ee:ff")
Expand Down Expand Up @@ -72,3 +74,37 @@ func TestNewInfoElementWithValue(t *testing.T) {
assert.Equal(t, element.GetInfoElement().Name, "sourceIPv4Address")
assert.Equal(t, element.GetIPAddressValue(), ip)
}

func BenchmarkEncodeInfoElementValueToBuffShortString(b *testing.B) {
// a short string has a max length of 254
str := strings.Repeat("x", 128)
element := NewStringInfoElement(NewInfoElement("interfaceDescription", 83, 13, 0, 65535), str)
const numCopies = 1000
length := element.GetLength()
buffer := make([]byte, numCopies*length)
b.ResetTimer()
for i := 0; i < b.N; i++ {
index := 0
for j := 0; j < numCopies; j++ {
require.NoError(b, encodeInfoElementValueToBuff(element, buffer, index))
index += length
}
}
}

func BenchmarkEncodeInfoElementValueToBuffLongString(b *testing.B) {
// a long string has a max length of 65535
str := strings.Repeat("x", 10000)
element := NewStringInfoElement(NewInfoElement("interfaceDescription", 83, 13, 0, 65535), str)
const numCopies = 1000
length := element.GetLength()
buffer := make([]byte, numCopies*length)
b.ResetTimer()
for i := 0; i < b.N; i++ {
index := 0
for j := 0; j < numCopies; j++ {
require.NoError(b, encodeInfoElementValueToBuff(element, buffer, index))
index += length
}
}
}

0 comments on commit b8729db

Please sign in to comment.