Send PacketOut when parseing failed

Signed-off-by: graysonwu <wgrayson@vmware.com>
antrea-io · Mar 15, 2023 · e4e9d01 · e4e9d01
1 parent 9cfee14
commit e4e9d01
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 6 deletions.
diff --git a/pkg/agent/controller/networkpolicy/fqdn.go b/pkg/agent/controller/networkpolicy/fqdn.go
@@ -749,7 +749,7 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 	handleDNSData := func(dnsData []byte) {
 		dnsMsg := dns.Msg{}
 		if err := dnsMsg.Unpack(dnsData); err != nil {
-			// A non-DNS response packet is received. Forward it to the Pod.
+			// A non-DNS response packet or a fragmented DNS response is received. Forward it to the Pod.
 			waitCh <- nil
 			return
 		}
@@ -758,6 +758,8 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 	go func() {
 		ethernetPkt, err := getEthernetPacket(pktIn)
 		if err != nil {
+			// Can't parse the packet. Forward it to the Pod.
+			waitCh <- nil
 			return
 		}
 		switch ipPkt := ethernetPkt.Data.(type) {
@@ -770,10 +772,14 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 			case protocol.Type_TCP:
 				tcpPkt, err := binding.GetTCPPacketFromIPMessage(ipPkt)
 				if err != nil {
+					// Can't parse the packet. Forward it to the Pod.
+					waitCh <- nil
 					return
 				}
 				dnsData, err := binding.GetTCPDNSData(tcpPkt)
 				if err != nil {
+					// A non-DNS response packet is received or a fragmented DNS response is received. Forward it to the Pod.
+					waitCh <- nil
 					return
 				}
 				handleDNSData(dnsData)
@@ -787,10 +793,14 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 			case protocol.Type_TCP:
 				tcpPkt, err := binding.GetTCPPacketFromIPMessage(ipPkt)
 				if err != nil {
+					// Can't parse the packet. Forward it to the Pod.
+					waitCh <- nil
 					return
 				}
 				dnsData, err := binding.GetTCPDNSData(tcpPkt)
 				if err != nil {
+					// A non-DNS response packet is received or a fragmented DNS response is received. Forward it to the Pod.
+					waitCh <- nil
 					return
 				}
 				handleDNSData(dnsData)
@@ -804,7 +814,7 @@ func (f *fqdnController) handlePacketIn(pktIn *ofctrl.PacketIn) error {
 		if err != nil {
 			return fmt.Errorf("error when syncing up rules for DNS reply, dropping packet: %v", err)
 		}
-		klog.V(2).InfoS("Rule sync is successful or not needed or a non-DNS response packet was received, forwarding the packet to Pod")
+		klog.V(2).InfoS("Rule sync is successful or not needed or a non-DNS response packet or a fragmented DNS response was received, forwarding the packet to Pod")
 		return f.sendDNSPacketout(pktIn)
 	}
 }

diff --git a/pkg/ovs/openflow/ofctrl_packetin.go b/pkg/ovs/openflow/ofctrl_packetin.go
@@ -79,8 +79,8 @@ func GetTCPDNSData(tcpPkt *protocol.TCP) (data []byte, err error) {
 	dnsDataLen := binary.BigEndian.Uint16(tcpPkt.Data[tcpOptionsLen : tcpOptionsLen+2])
 	dnsData := tcpPkt.Data[tcpOptionsLen+2:]
 	if int(dnsDataLen) > len(dnsData) {
-		klog.Info("DNS response has been fragmented")
-		return nil, fmt.Errorf("DNS response has been fragmented")
+		klog.Info("There is a non-DNS response or a fragmented DNS response in TCP payload")
+		return nil, fmt.Errorf("there is a non-DNS response or a fragmented DNS response in TCP payload")
 	}
 	return dnsData, nil
 }

diff --git a/pkg/ovs/openflow/ofctrl_packetin_test.go b/pkg/ovs/openflow/ofctrl_packetin_test.go
@@ -257,7 +257,7 @@ func TestGetTCPDNSData(t *testing.T) {
 					HdrLen: 6,
 					Data:   []byte{1, 2, 3, 4, 0, 2, 5},
 				},
-				expectErr:  fmt.Errorf("DNS response has been fragmented"),
+				expectErr:  fmt.Errorf("there is a non-DNS response or a fragmented DNS response in TCP payload"),
 				expectData: nil,
 			},
 		},

diff --git a/test/e2e/antreapolicy_test.go b/test/e2e/antreapolicy_test.go
@@ -3334,7 +3334,6 @@ func testFQDNPolicyTCP(t *testing.T) {
 		SetPriority(1.0).
 		SetAppliedToGroup([]ACNPAppliedToSpec{{NSSelector: map[string]string{}}})
 	builder.AddFQDNRule("github.com", ProtocolTCP, nil, nil, nil, "", nil, crdv1alpha1.RuleActionDrop)
-
 	testcases := []podToAddrTestStep{
 		{
 			Pod(namespaces["y"] + "/a"),