Differential D21240

net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST
ClosedPublic
Actions

Authored by jacob.e.keller_intel.com on Aug 12 2019, 10:53 PM.

Details

Reviewers

erj

Group Reviewers

Intel Networking

Commits

rS354345: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST

Summary

Some places in network code may need to verify that an ethernet address
is not the 'zero' address. Provide a standard macro ETHER_IS_ZERO for
this purpose, similar to the ETHER_IS_BROADCAST macro already available.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27346
Build 25597: arc lint + arc unit

Event Timeline

jacob.e.keller_intel.com created this revision.Aug 12 2019, 10:53 PM

Herald added a subscriber: ae. · View Herald TranscriptAug 12 2019, 10:53 PM

Harbormaster completed remote builds in B25817: Diff 60699.Aug 12 2019, 10:53 PM

afedorov added a subscriber: afedorov.Aug 13 2019, 7:42 PM

afedorov added inline comments.

sys/net/ethernet.h
79	Is this correct? If one byte of mac address is equal zero than the whole expression equal 0x00.

jacob.e.keller_intel.com added inline comments.Aug 13 2019, 9:19 PM

sys/net/ethernet.h
79	Yea, this is incorrect, woops. Should be bitwise or.

Use bitwise OR instead of bitwise AND to correct implementation

Harbormaster completed remote builds in B25855: Diff 60761.Aug 13 2019, 9:21 PM

It’s seems I found another incorrect solution:

#include <stdio.h>

int main()
{

unsigned char a = (1 << 0) ||(1<<1)||(1 <<2)||(1<<3)||(1<<4)||(1<<5);
printf("momentum more : %d\n", a);

return 0;

}

In D21240#462131, @aleksandr.fedorov_itglobal.com wrote:
It’s seems I found another incorrect solution:

#include <stdio.h>

int main()
{
unsigned char a = (1 << 0) ||(1<<1)||(1 <<2)||(1<<3)||(1<<4)||(1<<5);
printf("momentum more : %d\n", a);

return 0;
}

This uses boolean ||, not bitwise |

bitwise OR should correctly combine all of the chunks of the MAC address, and if any are non-zero, the whole thing will be non-zero.

Ops, you’re right. Sorry for the noise.

lwhsu added a subscriber: scottl.Sep 14 2019, 1:37 PM

jacob.e.keller_intel.com added reviewers: Intel Networking, erj.Oct 23 2019, 6:35 PM

erj accepted this revision.Nov 4 2019, 9:50 PM

This revision is now accepted and ready to land.Nov 4 2019, 9:50 PM

I get a build error with this revision:

--- if_ure.o ---
/root/srcs/head/src/sys/dev/usb/net/if_ure.c:71:9: error: 'ETHER_IS_ZERO' macro redefined [-Werror,-Wmacro-redefined]
#define ETHER_IS_ZERO(addr) \
        ^
/root/srcs/head/src/sys/net/ethernet.h:77:9: note: previous definition is here
#define ETHER_IS_ZERO(addr) \
        ^
1 error generated.

This revision now requires changes to proceed.Nov 4 2019, 9:57 PM

Remove ETHER_IS_ZERO definitions from USB networking drivers.

Harbormaster completed remote builds in B27346: Diff 63943.Nov 4 2019, 11:29 PM

Builds okay now.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 5 2019, 12:12 AM

Closed by commit rS354345: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST (authored by erj). · Explain Why

This revision was automatically updated to reflect the committed changes.

erj added a commit: rS354345: net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST.

Herald added a subscriber: imp. · View Herald TranscriptNov 5 2019, 12:12 AM

emaste mentioned this in D20079: [new driver] Microchips LAN743X driver for FreeBSD.Nov 6 2019, 7:46 PM

Revision Contents
Changeset List

Path

Size

sys/

dev/

usb/

net/

if_muge.c

3 lines

if_smsc.c

3 lines

if_ure.c

3 lines

net/

ethernet.h

3 lines

Diff 63943

View Options

sys/dev/usb/net/if_muge.c

View Options

sys/dev/usb/net/if_smsc.c

View Options

sys/dev/usb/net/if_ure.c

View Options

sys/net/ethernet.h

	Show First 20 Lines • Show All 68 Lines • ▼ Show 20 Lines
	struct ether_addr {			struct ether_addr {
	u_char octet[ETHER_ADDR_LEN];			u_char octet[ETHER_ADDR_LEN];
	} __packed;			} __packed;

	#define ETHER_IS_MULTICAST(addr) ((addr) & 0x01) / is address mcast/bcast? */			#define ETHER_IS_MULTICAST(addr) ((addr) & 0x01) / is address mcast/bcast? */
	#define ETHER_IS_BROADCAST(addr) \			#define ETHER_IS_BROADCAST(addr) \
	(((addr)[0] & (addr)[1] & (addr)[2] & \			(((addr)[0] & (addr)[1] & (addr)[2] & \
	(addr)[3] & (addr)[4] & (addr)[5]) == 0xff)			(addr)[3] & (addr)[4] & (addr)[5]) == 0xff)
				#define ETHER_IS_ZERO(addr) \
				(((addr)[0] \| (addr)[1] \| (addr)[2] \| \
				(addr)[3] \| (addr)[4] \| (addr)[5]) == 0x00)
				afedorovUnsubmitted Not Done Inline Actions Is this correct? If one byte of mac address is equal zero than the whole expression equal 0x00. afedorov: Is this correct? If one byte of mac address is equal zero than the whole expression equal 0x00.
				jacob.e.keller_intel.comAuthorUnsubmitted Done Inline Actions Yea, this is incorrect, woops. Should be bitwise or. jacob.e.keller_intel.com: Yea, this is incorrect, woops. Should be bitwise or.

	/*			/*
	* 802.1q Virtual LAN header.			* 802.1q Virtual LAN header.
	*/			*/
	struct ether_vlan_header {			struct ether_vlan_header {
	uint8_t evl_dhost[ETHER_ADDR_LEN];			uint8_t evl_dhost[ETHER_ADDR_LEN];
	uint8_t evl_shost[ETHER_ADDR_LEN];			uint8_t evl_shost[ETHER_ADDR_LEN];
	uint16_t evl_encap_proto;			uint16_t evl_encap_proto;
	▲ Show 20 Lines • Show All 371 Lines • Show Last 20 Lines

net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCASTClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 63943

sys/dev/usb/net/if_muge.c

sys/dev/usb/net/if_smsc.c

sys/dev/usb/net/if_ure.c

sys/net/ethernet.h

net: add ETHER_IS_ZERO macro similar to ETHER_IS_BROADCAST
ClosedPublic
Actions

Revision Contents
Changeset List